Bug 1191453 - Drop subscriptions for a site when the user revokes push permissions. r=mt,MattN
authorKit Cambridge <kcambridge@mozilla.com>
Tue, 06 Oct 2015 08:14:25 -0700
changeset 266460 cd18686b904dde3589b682fc934aba47f5cc2bb1
parent 266459 ac69d6a3ff2236452b9033d75e00121dc0729cb5
child 266461 906d018dd35818ee9fbf89d423cf67b9994bd871
push id29493
push userkwierso@gmail.com
push dateWed, 07 Oct 2015 17:31:17 +0000
treeherdermozilla-central@49d87bbe0122 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt, MattN
bugs1191453
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1191453 - Drop subscriptions for a site when the user revokes push permissions. r=mt,MattN
dom/push/PushDB.jsm
dom/push/PushRecord.jsm
dom/push/PushService.jsm
dom/push/PushServiceHttp2.jsm
dom/push/PushServiceWebSocket.jsm
dom/push/test/xpcshell/test_permissions.js
dom/push/test/xpcshell/test_quota_observer.js
dom/push/test/xpcshell/test_webapps_cleardata.js
dom/push/test/xpcshell/xpcshell.ini
--- a/dom/push/PushDB.jsm
+++ b/dom/push/PushDB.jsm
@@ -222,16 +222,45 @@ this.PushDB.prototype = {
           };
         },
         resolve,
         reject
       )
     );
   },
 
+  getAllByOrigin: function(origin, originAttributes) {
+    debug("getAllByOrigin()");
+
+    return new Promise((resolve, reject) =>
+      this.newTxn(
+        "readonly",
+        this._dbStoreName,
+        (aTxn, aStore) => {
+          aTxn.result = [];
+
+          let index = aStore.index("identifiers");
+          let range = IDBKeyRange.bound(
+            [origin, originAttributes],
+            [origin + "\x7f", originAttributes]
+          );
+          index.openCursor(range).onsuccess = event => {
+            let cursor = event.target.result;
+            if (cursor) {
+              aTxn.result.push(this.toPushRecord(cursor.value));
+              cursor.continue();
+            }
+          };
+        },
+        resolve,
+        reject
+      )
+    );
+  },
+
   // Perform a unique match against { scope, originAttributes }
   getByIdentifiers: function(aPageRecord) {
     debug("getByIdentifiers() { " + aPageRecord.scope + ", " +
           JSON.stringify(aPageRecord.originAttributes) + " }");
     if (!aPageRecord.scope || aPageRecord.originAttributes == undefined) {
       return Promise.reject(
                new TypeError("Scope and originAttributes are required! " +
                              JSON.stringify(aPageRecord)));
--- a/dom/push/PushRecord.jsm
+++ b/dom/push/PushRecord.jsm
@@ -45,18 +45,25 @@ function PushRecord(props) {
   this.p256dhPublicKey = props.p256dhPublicKey;
   this.p256dhPrivateKey = props.p256dhPrivateKey;
   this.setQuota(props.quota);
   this.ctime = (typeof props.ctime === "number") ? props.ctime : 0;
 }
 
 PushRecord.prototype = {
   setQuota(suggestedQuota) {
-    this.quota = (!isNaN(suggestedQuota) && suggestedQuota >= 0) ?
-                 suggestedQuota : prefs.get("maxQuotaPerSubscription");
+    if (!isNaN(suggestedQuota) && suggestedQuota >= 0) {
+      this.quota = suggestedQuota;
+    } else {
+      this.resetQuota();
+    }
+  },
+
+  resetQuota() {
+    this.quota = prefs.get("maxQuotaPerSubscription");
   },
 
   updateQuota(lastVisit) {
     if (this.isExpired() || !this.quotaApplies()) {
       // Ignore updates if the registration is already expired, or isn't
       // subject to quota.
       return;
     }
@@ -165,16 +172,21 @@ PushRecord.prototype = {
    * associated service worker.
    */
   hasPermission() {
     let permission = Services.perms.testExactPermissionFromPrincipal(
       this.principal, "desktop-notification");
     return permission == Ci.nsIPermissionManager.ALLOW_ACTION;
   },
 
+  quotaChanged() {
+    return this.getLastVisit()
+      .then(lastVisit => lastVisit > this.lastPush);
+  },
+
   quotaApplies() {
     return Number.isFinite(this.quota);
   },
 
   isExpired() {
     return this.quota === 0;
   },
 
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -256,68 +256,66 @@ this.PushService = {
           gDebuggingEnabled = prefs.get("debug");
         }
         break;
 
       case "idle-daily":
         this._dropExpiredRegistrations();
         break;
 
+      case "perm-changed":
+        this._onPermissionChange(aSubject, aData).catch(error => {
+          debug("onPermissionChange: Error updating registrations: " +
+            error);
+        })
+        break;
+
       case "webapps-clear-data":
         debug("webapps-clear-data");
 
         let data = aSubject
                      .QueryInterface(Ci.mozIApplicationClearPrivateDataParams);
         if (!data) {
           debug("webapps-clear-data: Failed to get information about " +
                 "application");
           return;
         }
 
         var originAttributes =
           ChromeUtils.originAttributesToSuffix({ appId: data.appId,
                                                  inBrowser: data.browserOnly });
         this._db.getAllByOriginAttributes(originAttributes)
-          .then(records => {
-            records.forEach(record => {
-              this._db.delete(record.keyID)
-                .then(_ => {
-                  // courtesy, but don't establish a connection
-                  // just for it
-                  if (this._ws) {
-                    debug("Had a connection, so telling the server");
-                    this._sendUnregister({channelID: record.channelID})
-                        .catch(function(e) {
-                          debug("Unregister errored " + e);
-                        });
-                  }
-                }, err => {
-                  debug("webapps-clear-data: " + record.scope +
-                        " Could not delete entry " + record.channelID);
+          .then(records => Promise.all(records.map(record =>
+            this._db.delete(record.keyID).then(
+              _ => this._unregisterIfConnected(record),
+              err => {
+                debug("webapps-clear-data: " + record.scope +
+                      " Could not delete entry " + record.channelID);
 
-                  // courtesy, but don't establish a connection
-                  // just for it
-                  if (this._ws) {
-                    debug("Had a connection, so telling the server");
-                    this._sendUnregister({channelID: record.channelID})
-                        .catch(function(e) {
-                          debug("Unregister errored " + e);
-                        });
-                  }
-                  throw "Database error";
-                });
-            });
-          }, _ => {
-            debug("webapps-clear-data: Error in getAllByOriginAttributes(" + originAttributes + ")");
-          });
+                return this._unregisterIfConnected(record);
+              })
+            )
+          ));
 
         break;
     }
   },
 
+  _unregisterIfConnected: function(record) {
+    if (this._service.isConnected()) {
+      // courtesy, but don't establish a connection
+      // just for it
+      debug("Had a connection, so telling the server");
+      return this._sendUnregister({channelID: record.channelID})
+          .catch(function(e) {
+            debug("Unregister errored " + e);
+          });
+    }
+  },
+
   // utility function used to add/remove observers in startObservers() and
   // stopObservers()
   getNetworkStateChangeEventName: function() {
     try {
       Cc["@mozilla.org/network/manager;1"].getService(Ci.nsINetworkManager);
       return "network-active-changed";
     } catch (e) {
       return "network:offline-status-changed";
@@ -494,18 +492,22 @@ this.PushService = {
     // online, so we explicitly check for the presence of NetworkManager and
     // don't add an observer for offline-status-changed on B2G.
     this._networkStateChangeEventName = this.getNetworkStateChangeEventName();
     Services.obs.addObserver(this, this._networkStateChangeEventName, false);
 
     // Used to monitor if the user wishes to disable Push.
     prefs.observe("connection.enabled", this);
 
-    // Used to prune expired registrations and notify dormant service workers.
+    // Prunes expired registrations and notifies dormant service workers.
     Services.obs.addObserver(this, "idle-daily", false);
+
+    // Prunes registrations for sites for which the user revokes push
+    // permissions.
+    Services.obs.addObserver(this, "perm-changed", false);
   },
 
   _startService: function(service, serverURI, event, options = {}) {
     debug("startService");
 
     if (this._state != PUSH_SERVICE_ACTIVATING) {
       return;
     }
@@ -525,17 +527,17 @@ this.PushService = {
 
     this._db = options.db;
     if (!this._db) {
       this._db = this._service.newPushDB();
     }
 
     this._service.init(options, this, serverURI);
     this._startObservers();
-    return Promise.resolve();
+    return this._dropExpiredRegistrations();
   },
 
   /**
    * PushService uninitialization is divided into 3 parts:
    * stopObservers() - stot observers started in startObservers.
    * stopService() - It stops listening for broadcasted messages, stops db and
    *                 PushService connection (WebSocket).
    *                 state is changed to PUSH_SERVICE_INIT.
@@ -594,16 +596,17 @@ this.PushService = {
     }
 
     prefs.ignore("debug", this);
     prefs.ignore("connection.enabled", this);
 
     Services.obs.removeObserver(this, this._networkStateChangeEventName);
     Services.obs.removeObserver(this, "webapps-clear-data");
     Services.obs.removeObserver(this, "idle-daily");
+    Services.obs.removeObserver(this, "perm-changed");
   },
 
   uninit: function() {
     debug("uninit()");
 
     this._childListeners = [];
 
     if (this._state == PUSH_SERVICE_UNINIT) {
@@ -753,16 +756,21 @@ this.PushService = {
   updateRecordAndNotifyApp: function(aKeyID, aUpdateFunc) {
     return this._db.update(aKeyID, aUpdateFunc)
       .then(record => {
         this._notifySubscriptionChangeObservers(record);
         return record;
       });
   },
 
+  dropRecordAndNotifyApp: function(aRecord) {
+    return this._db.delete(aRecord.keyID)
+      .then(_ => this._notifySubscriptionChangeObservers(aRecord));
+  },
+
   _recordDidNotNotify: function(reason) {
     Services.telemetry.
       getHistogramById("PUSH_API_NOTIFICATION_RECEIVED_BUT_DID_NOT_NOTIFY").
       add(reason);
   },
 
   /**
    * Dispatches an incoming message to a service worker, recalculating the
@@ -946,26 +954,24 @@ this.PushService = {
 
     return this._checkActivated()
       .then(_ => this._db.getByIdentifiers(aPageRecord))
       .then(record => {
         if (!record) {
           return this._lookupOrPutPendingRequest(aPageRecord);
         }
         if (record.isExpired()) {
-          return record.getLastVisit().then(lastVisit => {
-            if (lastVisit > record.lastPush) {
+          return record.quotaChanged().then(isChanged => {
+            if (isChanged) {
               // If the user revisited the site, drop the expired push
               // registration and re-register.
-              return this._db.delete(record.keyID).then(_ => {
-                return this._lookupOrPutPendingRequest(aPageRecord);
-              });
+              return this._db.delete(record.keyID);
             }
             throw {state: 0, error: "NotFoundError"};
-          });
+          }).then(_ => this._lookupOrPutPendingRequest(aPageRecord));
         }
         return record;
       }, error => {
         debug("getByIdentifiers failed");
         throw error;
       });
   },
 
@@ -1216,22 +1222,22 @@ this.PushService = {
 
     return this._checkActivated()
       .then(_ => this._db.getByIdentifiers(aPageRecord))
       .then(record => {
         if (!record) {
           return null;
         }
         if (record.isExpired()) {
-          return record.getLastVisit().then(lastVisit => {
-            if (lastVisit > record.lastPush) {
-              return this._db.delete(record.keyID).then(_ => null);
+          return record.quotaChanged().then(isChanged => {
+            if (isChanged) {
+              return this._db.delete(record.keyID);
             }
             throw {state: 0, error: "NotFoundError"};
-          });
+          }).then(_ => null);
         }
         return record.toRegistration();
       });
   },
 
   registration: function(aPageRecord, aMessageManager) {
     debug("registration()");
 
@@ -1246,29 +1252,135 @@ this.PushService = {
           error
         })
       );
   },
 
   _dropExpiredRegistrations: function() {
     debug("dropExpiredRegistrations()");
 
-    this._db.getAllExpired().then(records => {
-      return Promise.all(records.map(record => {
-        return record.getLastVisit().then(lastVisit => {
-          if (lastVisit > record.lastPush) {
+    return this._db.getAllExpired().then(records => {
+      return Promise.all(records.map(record =>
+        record.quotaChanged().then(isChanged => {
+          if (isChanged) {
             // If the user revisited the site, drop the expired push
             // registration and notify the associated service worker.
-            return this._db.delete(record.keyID).then(() => {
-              this._notifySubscriptionChangeObservers(record);
-            });
+            return this.dropRecordAndNotifyApp(record);
           }
         }).catch(error => {
           debug("dropExpiredRegistrations: Error dropping registration " +
             record.keyID + ": " + error);
-        });
-      }));
+        })
+      ));
+    });
+  },
+
+  _onPermissionChange: function(subject, data) {
+    debug("onPermissionChange()");
+
+    if (data == "cleared") {
+      // If the permission list was cleared, drop all registrations
+      // that are subject to quota.
+      return this._db.clearIf(record => {
+        if (record.quotaApplies()) {
+          if (!record.isExpired()) {
+            // Drop the registration in the background.
+            this._unregisterIfConnected(record);
+          }
+          return true;
+        }
+        return false;
+      });
+    }
+
+    let permission = subject.QueryInterface(Ci.nsIPermission);
+    if (permission.type != "push") {
+      return Promise.resolve();
+    }
+
+    return this._updatePermission(permission, data);
+  },
+
+  _updatePermission: function(permission, type) {
+    debug("updatePermission()");
+
+    let isAllow = permission.capability ==
+                  Ci.nsIPermissionManager.ALLOW_ACTION;
+    let isChange = type == "added" || type == "changed";
+
+    if (isAllow && isChange) {
+      // Permission set to "allow". Drop all expired registrations for this
+      // site, notify the associated service workers, and reset the quota
+      // for active registrations.
+      return this._getByPrincipal(permission.principal)
+        .then(records => this._permissionAllowed(records));
+    } else if (isChange || (isAllow && type == "deleted")) {
+      // Permission set to "block" or "always ask," or "allow" permission
+      // removed. Expire all registrations for this site.
+      return this._getByPrincipal(permission.principal)
+        .then(records => this._permissionDenied(records));
+    }
+
+    return Promise.resolve();
+  },
+
+  _getByPrincipal: function(principal) {
+    return this._db.getAllByOrigin(
+      principal.URI.prePath,
+      ChromeUtils.originAttributesToSuffix(principal.originAttributes)
+    );
+  },
+
+  /**
+   * Expires all registrations if the push permission is revoked. We only
+   * expire the registration so we can notify the service worker as soon as
+   * the permission is reinstated. If we just deleted the registration, the
+   * worker wouldn't be notified until the next visit to the site.
+   *
+   * @param {Array} A list of records to expire.
+   * @returns {Promise} A promise resolved with the expired records.
+   */
+  _permissionDenied: function(records) {
+    return Promise.all(records.filter(record =>
+      // Ignore already-expired records.
+      record.quotaApplies() && !record.isExpired()
+    ).map(record =>
+      this._expireRegistration(record)
+    ));
+  },
+
+  /**
+   * Drops all expired registrations, notifies the associated service
+   * workers, and resets the quota for active registrations if the push
+   * permission is granted.
+   *
+   * @param {Array} A list of records to refresh.
+   * @returns {Promise} A promise resolved with the refreshed records.
+   */
+  _permissionAllowed: function(records) {
+    return Promise.all(records.map(record => {
+      if (!record.quotaApplies()) {
+        return record;
+      }
+      if (record.isExpired()) {
+        // If the registration has expired, drop and notify the worker
+        // unconditionally.
+        return this.dropRecordAndNotifyApp(record);
+      }
+      return this._db.update(record.keyID, record => {
+        record.resetQuota();
+        return record;
+      });
+    }));
+  },
+
+  _expireRegistration: function(record) {
+    // Drop the registration in the background.
+    this._unregisterIfConnected(record);
+    return this._db.update(record.keyID, record => {
+      record.setQuota(0);
+      return record;
     }).catch(error => {
-      debug("dropExpiredRegistrations: Error dropping registrations: " +
-        error);
+      debug("expireRegistration: Error dropping expired registration " +
+        record.keyID + ": " + error);
     });
   },
 };
--- a/dom/push/PushServiceHttp2.jsm
+++ b/dom/push/PushServiceHttp2.jsm
@@ -474,16 +474,20 @@ this.PushServiceHttp2 = {
       }
     }
   },
 
   connect: function(subscriptions) {
     this.startConnections(subscriptions);
   },
 
+  isConnected: function() {
+    return this._mainPushService != null;
+  },
+
   disconnect: function() {
     this._shutdownConnections(false);
   },
 
   _makeChannel: function(aUri) {
 
     var ios = Cc["@mozilla.org/network/io-service;1"]
                 .getService(Ci.nsIIOService);
--- a/dom/push/PushServiceWebSocket.jsm
+++ b/dom/push/PushServiceWebSocket.jsm
@@ -644,16 +644,20 @@ this.PushServiceWebSocket = {
   connect: function(records) {
     debug("connect");
     // Check to see if we need to do anything.
     if (records.length > 0) {
       this._beginWSSetup();
     }
   },
 
+  isConnected: function() {
+    return !!this._ws;
+  },
+
   /**
    * There is only one alarm active at any time. This alarm has 3 intervals
    * corresponding to 3 tasks.
    *
    * 1) Reconnect on ping timeout.
    *    If we haven't received any messages from the server by the time this
    *    alarm fires, the connection is closed and PushService tries to
    *    reconnect, repurposing the alarm for (3).
new file mode 100644
--- /dev/null
+++ b/dom/push/test/xpcshell/test_permissions.js
@@ -0,0 +1,268 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+'use strict';
+
+const {PushDB, PushService, PushServiceWebSocket} = serviceExports;
+
+const userAgentID = '2c43af06-ab6e-476a-adc4-16cbda54fb89';
+
+let db;
+
+function run_test() {
+  do_get_profile();
+  setPrefs({
+    userAgentID,
+  });
+
+  db = PushServiceWebSocket.newPushDB();
+  do_register_cleanup(() => {return db.drop().then(_ => db.close());});
+
+  run_next_test();
+}
+
+let unregisterDefers = {};
+
+function putRecord(channelID, scope, quota) {
+  return db.put({
+    channelID: channelID,
+    pushEndpoint: 'https://example.org/push/' + channelID,
+    scope: scope,
+    pushCount: 0,
+    lastPush: 0,
+    version: null,
+    originAttributes: '',
+    quota: quota,
+  });
+}
+
+function makePushPermission(url, capability) {
+  return {
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIPermission]),
+    capability: Ci.nsIPermissionManager[capability],
+    expireTime: 0,
+    expireType: Ci.nsIPermissionManager.EXPIRE_NEVER,
+    principal: Services.scriptSecurityManager.getCodebasePrincipal(
+      Services.io.newURI(url, null, null)
+    ),
+    type: 'push',
+  };
+}
+
+function promiseSubscriptionChanges(count) {
+  let notifiedScopes = [];
+  let subChangePromise = promiseObserverNotification('push-subscription-change', (subject, data) => {
+    notifiedScopes.push(data);
+    return notifiedScopes.length == count;
+  });
+  return subChangePromise.then(_ => notifiedScopes.sort());
+}
+
+function allExpired(...keyIDs) {
+  return Promise.all(keyIDs.map(
+    keyID => db.getByKeyID(keyID)
+  )).then(records =>
+    records.every(record => record.isExpired())
+  );
+}
+
+add_task(function* setUp() {
+  // Active registration; quota should be reset to 16. Since the quota isn't
+  // exposed to content, we shouldn't receive a subscription change event.
+  yield putRecord('active-allow', 'https://example.info/page/1', 8);
+
+  // Expired registration; should be dropped.
+  yield putRecord('expired-allow', 'https://example.info/page/2', 0);
+
+  // Active registration; should be expired when we change the permission
+  // to "deny".
+  yield putRecord('active-deny-changed', 'https://example.xyz/page/1', 16);
+
+  // Two active registrations for a visited site. These will expire when we
+  // add a "deny" permission.
+  yield putRecord('active-deny-added-1', 'https://example.net/ham', 16);
+  yield putRecord('active-deny-added-2', 'https://example.net/green', 8);
+
+  // An already-expired registration for a visited site. We shouldn't send an
+  // `unregister` request for this one, but still receive an observer
+  // notification when we restore permissions.
+  yield putRecord('expired-deny-added', 'https://example.net/eggs', 0);
+
+  // A registration that should not be affected by permission list changes
+  // because its quota is set to `Infinity`.
+  yield putRecord('never-expires', 'app://chrome/only', Infinity);
+
+  // A registration that should be dropped when we clear the permission
+  // list.
+  yield putRecord('drop-on-clear', 'https://example.edu/lonely', 16);
+
+  let handshakeDone;
+  let handshakePromise = new Promise(resolve => handshakeDone = resolve);
+  PushService.init({
+    serverURI: 'wss://push.example.org/',
+    networkInfo: new MockDesktopNetworkInfo(),
+    db,
+    makeWebSocket(uri) {
+      return new MockWebSocket(uri, {
+        onHello(request) {
+          this.serverSendMsg(JSON.stringify({
+            messageType: 'hello',
+            status: 200,
+            uaid: userAgentID,
+          }));
+          handshakeDone();
+        },
+        onUnregister(request) {
+          let resolve = unregisterDefers[request.channelID];
+          equal(typeof resolve, 'function',
+            'Dropped unexpected channel ID ' + request.channelID);
+          delete unregisterDefers[request.channelID];
+          resolve();
+        },
+        onACK(request) {},
+      });
+    }
+  });
+  yield waitForPromise(handshakePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for handshake');
+});
+
+add_task(function* test_permissions_allow_added() {
+  let subChangePromise = promiseSubscriptionChanges(1);
+
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.info', 'ALLOW_ACTION'),
+    'added'
+  );
+  let notifiedScopes = yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+      'Timed out waiting for notifications after adding allow');
+
+  deepEqual(notifiedScopes, [
+    'https://example.info/page/2',
+  ], 'Wrong scopes after adding allow');
+
+  let record = yield db.getByKeyID('active-allow');
+  equal(record.quota, 16,
+    'Should reset quota for active records after adding allow');
+
+  record = yield db.getByKeyID('expired-allow');
+  ok(!record, 'Should drop expired records after adding allow');
+});
+
+add_task(function* test_permissions_allow_deleted() {
+  let unregisterPromise = new Promise(resolve => unregisterDefers[
+    'active-allow'] = resolve);
+
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.info', 'ALLOW_ACTION'),
+    'deleted'
+  );
+
+  yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for unregister after deleting allow');
+
+  let record = yield db.getByKeyID('active-allow');
+  ok(record.isExpired(),
+    'Should expire active record after deleting allow');
+});
+
+add_task(function* test_permissions_deny_added() {
+  let unregisterPromise = Promise.all([
+    new Promise(resolve => unregisterDefers[
+      'active-deny-added-1'] = resolve),
+    new Promise(resolve => unregisterDefers[
+      'active-deny-added-2'] = resolve),
+  ]);
+
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.net', 'DENY_ACTION'),
+    'added'
+  );
+  yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for notifications after adding deny');
+
+  let isExpired = yield allExpired(
+    'active-deny-added-1',
+    'expired-deny-added'
+  );
+  ok(isExpired, 'Should expire all registrations after adding deny');
+});
+
+add_task(function* test_permissions_deny_deleted() {
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.net', 'DENY_ACTION'),
+    'deleted'
+  );
+
+  let isExpired = yield allExpired(
+    'active-deny-added-1',
+    'expired-deny-added'
+  );
+  ok(isExpired, 'Should retain expired registrations after deleting deny');
+});
+
+add_task(function* test_permissions_allow_changed() {
+  let subChangePromise = promiseSubscriptionChanges(3);
+
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.net', 'ALLOW_ACTION'),
+    'changed'
+  );
+
+  let notifiedScopes = yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for notifications after changing to allow');
+
+  deepEqual(notifiedScopes, [
+    'https://example.net/eggs',
+    'https://example.net/green',
+    'https://example.net/ham'
+  ], 'Wrong scopes after changing to allow');
+
+  let droppedRecords = yield Promise.all([
+    db.getByKeyID('active-deny-added-1'),
+    db.getByKeyID('active-deny-added-2'),
+    db.getByKeyID('expired-deny-added'),
+  ]);
+  ok(!droppedRecords.some(Boolean),
+    'Should drop all expired registrations after changing to allow');
+});
+
+add_task(function* test_permissions_deny_changed() {
+  let unregisterPromise = new Promise(resolve => unregisterDefers[
+    'active-deny-changed'] = resolve);
+
+  yield PushService._onPermissionChange(
+    makePushPermission('https://example.xyz', 'DENY_ACTION'),
+    'changed'
+  );
+
+  yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for unregister after changing to deny');
+
+  let record = yield db.getByKeyID('active-deny-changed');
+  ok(record.isExpired(),
+    'Should expire active record after changing to allow');
+});
+
+add_task(function* test_permissions_clear() {
+  let records = yield db.getAllKeyIDs();
+  deepEqual(records.map(record => record.keyID).sort(), [
+    'active-allow',
+    'active-deny-changed',
+    'drop-on-clear',
+    'never-expires',
+  ], 'Wrong records in database before clearing');
+
+  let unregisterPromise = new Promise(resolve => unregisterDefers[
+      'drop-on-clear'] = resolve);
+
+  yield PushService._onPermissionChange(null, 'cleared');
+
+  yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for unregister requests after clearing permissions');
+
+  records = yield db.getAllKeyIDs();
+  deepEqual(records.map(record => record.keyID).sort(), [
+    'never-expires',
+  ], 'Unrestricted registrations should not be dropped');
+});
--- a/dom/push/test/xpcshell/test_quota_observer.js
+++ b/dom/push/test/xpcshell/test_quota_observer.js
@@ -26,21 +26,21 @@ add_task(function* test_expiration_histo
     scope: 'https://example.com/deals',
     pushCount: 0,
     lastPush: 0,
     version: null,
     originAttributes: '',
     quota: 16,
   });
 
-  // ...And an expired registration that we'll revive later.
+  // ...And a registration that we'll evict on startup.
   yield db.put({
-    channelID: 'eb33fc90-c883-4267-b5cb-613969e8e349',
-    pushEndpoint: 'https://example.org/push/2',
-    scope: 'https://example.com/auctions',
+    channelID: '4cb6e454-37cf-41c4-a013-4e3a7fdd0bf1',
+    pushEndpoint: 'https://example.org/push/3',
+    scope: 'https://example.com/stuff',
     pushCount: 0,
     lastPush: 0,
     version: null,
     originAttributes: '',
     quota: 0,
   });
 
   yield addVisit({
@@ -49,16 +49,18 @@ add_task(function* test_expiration_histo
     visits: [{
       visitDate: (Date.now() - 14 * 24 * 60 * 60 * 1000) * 1000,
       transitionType: Ci.nsINavHistoryService.TRANSITION_LINK,
     }],
   });
 
   let unregisterDone;
   let unregisterPromise = new Promise(resolve => unregisterDone = resolve);
+  let subChangePromise = promiseObserverNotification('push-subscription-change', (subject, data) =>
+    data == 'https://example.com/stuff');
 
   PushService.init({
     serverURI: 'wss://push.example.org/',
     networkInfo: new MockDesktopNetworkInfo(),
     db,
     makeWebSocket(uri) {
       return new MockWebSocket(uri, {
         onHello(request) {
@@ -82,28 +84,42 @@ add_task(function* test_expiration_histo
           equal(request.channelID, '379c0668-8323-44d2-a315-4ee83f1a9ee9', 'Dropped wrong channel ID');
           unregisterDone();
         },
         onACK(request) {},
       });
     }
   });
 
+  yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for subscription change event on startup');
   yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
     'Timed out waiting for unregister request');
 
   let expiredRecord = yield db.getByKeyID('379c0668-8323-44d2-a315-4ee83f1a9ee9');
   strictEqual(expiredRecord.quota, 0, 'Expired record not updated');
 
   let notifiedScopes = [];
-  let subChangePromise = promiseObserverNotification('push-subscription-change', (subject, data) => {
+  subChangePromise = promiseObserverNotification('push-subscription-change', (subject, data) => {
     notifiedScopes.push(data);
     return notifiedScopes.length == 2;
   });
 
+  // Add an expired registration that we'll revive later.
+  yield db.put({
+    channelID: 'eb33fc90-c883-4267-b5cb-613969e8e349',
+    pushEndpoint: 'https://example.org/push/2',
+    scope: 'https://example.com/auctions',
+    pushCount: 0,
+    lastPush: 0,
+    version: null,
+    originAttributes: '',
+    quota: 0,
+  });
+
   // Now visit the site...
   yield addVisit({
     uri: 'https://example.com/another-page',
     title: 'Infrequently-visited page',
     visits: [{
       visitDate: Date.now() * 1000,
       transitionType: Ci.nsINavHistoryService.TRANSITION_LINK,
     }],
--- a/dom/push/test/xpcshell/test_webapps_cleardata.js
+++ b/dom/push/test/xpcshell/test_webapps_cleardata.js
@@ -21,16 +21,19 @@ function run_test() {
   );
   run_next_test();
 }
 
 add_task(function* test_webapps_cleardata() {
   let db = PushServiceWebSocket.newPushDB();
   do_register_cleanup(() => {return db.drop().then(_ => db.close());});
 
+  let unregisterDone;
+  let unregisterPromise = new Promise(resolve => unregisterDone = resolve);
+
   PushService.init({
     serverURI: "wss://push.example.org",
     networkInfo: new MockDesktopNetworkInfo(),
     db,
     makeWebSocket(uri) {
       return new MockWebSocket(uri, {
         onHello(data) {
           equal(data.messageType, 'hello', 'Handshake: wrong message type');
@@ -45,17 +48,20 @@ add_task(function* test_webapps_cleardat
           equal(data.messageType, 'register', 'Register: wrong message type');
           this.serverSendMsg(JSON.stringify({
             messageType: 'register',
             status: 200,
             channelID: data.channelID,
             uaid: userAgentID,
             pushEndpoint: 'https://example.com/update/' + Math.random(),
           }));
-        }
+        },
+        onUnregister(data) {
+          unregisterDone();
+        },
       });
     }
   });
 
   let registers = yield Promise.all([
     PushNotificationService.register(
       'https://example.org/1',
       ChromeUtils.originAttributesToSuffix({ appId: 1, inBrowser: false })),
@@ -79,10 +85,13 @@ add_task(function* test_webapps_cleardat
     'https://example.org/1',
     ChromeUtils.originAttributesToSuffix({ appId: 1, inBrowser: false }));
   ok(!registration, 'Registration for { 1, false } should not exist.');
 
   registration = yield PushNotificationService.registration(
     'https://example.org/1',
     ChromeUtils.originAttributesToSuffix({ appId: 1, inBrowser: true }));
   ok(registration, 'Registration for { 1, true } should still exist.');
+
+  yield waitForPromise(unregisterPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for unregister');
 });
 
--- a/dom/push/test/xpcshell/xpcshell.ini
+++ b/dom/push/test/xpcshell/xpcshell.ini
@@ -4,16 +4,20 @@ tail =
 # Push notifications and alarms are currently disabled on Android.
 skip-if = toolkit == 'android'
 
 [test_notification_ack.js]
 [test_notification_duplicate.js]
 [test_notification_error.js]
 [test_notification_incomplete.js]
 [test_notification_version_string.js]
+
+[test_permissions.js]
+run-sequentially = This will delete all existing push subscriptions.
+
 [test_quota_exceeded.js]
 [test_quota_observer.js]
 [test_register_case.js]
 [test_register_flush.js]
 [test_register_invalid_channel.js]
 [test_register_invalid_endpoint.js]
 [test_register_invalid_json.js]
 [test_register_no_id.js]
@@ -55,9 +59,9 @@ run-sequentially = node server exception
 [test_registration_success_http2.js]
 skip-if = !hasNode
 run-sequentially = node server exceptions dont replay well
 [test_registration_error_http2.js]
 skip-if = !hasNode
 run-sequentially = node server exceptions dont replay well
 [test_clearAll_successful.js]
 skip-if = !hasNode
-run-sequentially = This will delete all existing push subscritions.
+run-sequentially = This will delete all existing push subscriptions.