Bug 1260519 - Support unsubscribing from Push API messages from a site. r=jchen
authorNick Alexander <nalexander@mozilla.com>
Tue, 29 Mar 2016 11:59:52 -0700
changeset 290899 1b9a489995479fb498c61857cc9311d81a88257e
parent 290898 34fabc54b7a4ad90dade559d0678a1a1ddc21536
child 290900 e14db462d31d566570e3bece66d5380f7b1ad400
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjchen
bugs1260519
milestone48.0a1
Bug 1260519 - Support unsubscribing from Push API messages from a site. r=jchen This was implemented, but never wired up. I thought long and hard about how to unit test this, and it's quite difficult. First, we'd have to chose a layer of testing. We could unit test: * the JS <-> Java message passing; * the permission prompts <-> JS interface; * some interactions with the Service Worker interface. The first is difficult because none of our current testing emulators have Google Play Services and GCM enabled, so we'd need to allow to mock or otherwise fake the GCM registration. Then we'd need to stand up a mock autopush server (using httpd.js or the Java-side equivalent), or mock out the autopush client as well. At this point, we're testing sendMessage. This could be done, but I'd rather slide this fix in before building out quite a bit of test infrastructure. (For the record, the Java Push Service state machine is thoroughly tested with Java unit tests, so I have confidence that the unsubscribe logic works.) The second is tested via the PushWebSocket tests, which are now running on Android. That is, if permissions and the PushService are interacting badly, we should see it with the existing test suite. Since PushServiceAndroidGCM is pretty much a pass-through, there's little value to be added here. Finally, the third is also tested via the PushWebSocket tests. There's absolutely nothing GCM specific about the Service Worker interface to the PushService. So I'm left manually testing this -- and now we can unsubscribe from Push messages from sites. MozReview-Commit-ID: HiRiqasHJ27
mobile/android/base/java/org/mozilla/gecko/push/PushService.java
--- a/mobile/android/base/java/org/mozilla/gecko/push/PushService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/push/PushService.java
@@ -208,25 +208,27 @@ public class PushService implements Bund
             Log.e(LOG_TAG, "callback must not be null in " + event);
             return;
         }
 
         try {
             if ("PushServiceAndroidGCM:Configure".equals(event)) {
                 final String endpoint = message.getString("endpoint");
                 if (endpoint == null) {
-                    Log.e(LOG_TAG, "endpoint must not be null in " + event);
+                    callback.sendError("endpoint must not be null in " + event);
                     return;
                 }
                 final boolean debug = message.getBoolean("debug", false);
                 pushManager.configure(geckoProfile.getName(), endpoint, debug, System.currentTimeMillis()); // For side effects.
                 callback.sendSuccess(null);
                 return;
             }
             if ("PushServiceAndroidGCM:DumpRegistration".equals(event)) {
+                // In the future, this might be used to interrogate the Java Push Manager
+                // registration state from JavaScript.
                 callback.sendError("Not yet implemented!");
                 return;
             }
             if ("PushServiceAndroidGCM:DumpSubscriptions".equals(event)) {
                 try {
                     final Map<String, PushSubscription> result = pushManager.allSubscriptionsForProfile(geckoProfile.getName());
 
                     final JSONObject json = new JSONObject();
@@ -245,16 +247,20 @@ public class PushService implements Bund
                     callback.sendSuccess(null);
                 } catch (PushManager.ProfileNeedsConfigurationException | AutopushClientException | PushClient.LocalException | IOException e) {
                     Log.e(LOG_TAG, "Got exception in " + event, e);
                     callback.sendError("Got exception handling message [" + event + "]: " + e.toString());
                 }
                 return;
             }
             if ("PushServiceAndroidGCM:UnregisterUserAgent".equals(event)) {
+                // In the future, this might be used to tell the Java Push Manager to unregister
+                // a User Agent entirely from JavaScript.  Right now, however, everything is
+                // subscription based; there's no concept of unregistering all subscriptions
+                // simultaneously.
                 callback.sendError("Not yet implemented!");
                 return;
             }
             if ("PushServiceAndroidGCM:SubscribeChannel".equals(event)) {
                 final String service = SERVICE_WEBPUSH;
                 final JSONObject serviceData;
                 try {
                     serviceData = new JSONObject();
@@ -283,17 +289,30 @@ public class PushService implements Bund
                     Log.e(LOG_TAG, "Got exception in " + event, e);
                     callback.sendError("Got exception handling message [" + event + "]: " + e.toString());
                     return;
                 }
                 callback.sendSuccess(json);
                 return;
             }
             if ("PushServiceAndroidGCM:UnsubscribeChannel".equals(event)) {
-                callback.sendError("Not yet implemented!");
+                final String channelID = message.getString("channelID");
+                if (channelID == null) {
+                    callback.sendError("channelID must not be null in " + event);
+                    return;
+                }
+
+                // Fire and forget.  See comments in the function itself.
+                final PushSubscription pushSubscription = pushManager.unsubscribeChannel(channelID);
+                if (pushSubscription != null) {
+                    callback.sendSuccess(null);
+                    return;
+                }
+
+                callback.sendError("Could not unsubscribe from channel: " + channelID);
                 return;
             }
         } catch (GcmTokenClient.NeedsGooglePlayServicesException e) {
             // TODO: improve this.  Can we find a point where the user is *definitely* interacting
             // with the WebPush?  Perhaps we can show a dialog when interacting with the Push
             // permissions, and then be more aggressive showing this notification when we have
             // registrations and subscriptions that can't be advanced.
             callback.sendError("To handle event [" + event + "], user interaction is needed to enable Google Play Services.");