Bug 1223202 - Only send subscription change events if the Push permission is granted. r=mt a=ritu
authorKit Cambridge <kcambridge@mozilla.com>
Tue, 10 Nov 2015 14:27:47 -0800
changeset 305903 c0b161f60c6b037555b97bab0e084ec9945945dc
parent 305902 4413def26994c535d29add8892df5cc50343c630
child 305904 a7e4b6ab821f03dba1d7b9b810e5604456722ba7
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt, ritu
bugs1223202
milestone44.0
Bug 1223202 - Only send subscription change events if the Push permission is granted. r=mt a=ritu
dom/push/PushDB.jsm
dom/push/PushRecord.jsm
dom/push/PushService.jsm
dom/push/PushServiceHttp2.jsm
dom/push/PushServiceWebSocket.jsm
dom/push/test/xpcshell/head.js
dom/push/test/xpcshell/test_drop_expired.js
dom/push/test/xpcshell/test_notification_incomplete.js
dom/push/test/xpcshell/test_quota_observer.js
dom/push/test/xpcshell/xpcshell.ini
--- a/dom/push/PushDB.jsm
+++ b/dom/push/PushDB.jsm
@@ -126,17 +126,20 @@ this.PushDB.prototype = {
     debug("delete()");
 
     return new Promise((resolve, reject) =>
       this.newTxn(
         "readwrite",
         this._dbStoreName,
         function txnCb(aTxn, aStore) {
           debug("Going to delete " + aKeyID);
-          aStore.delete(aKeyID);
+          aStore.get(aKeyID).onsuccess = event => {
+            aTxn.result = event.target.result;
+            aStore.delete(aKeyID);
+          };
         },
         resolve,
         reject
       )
     );
   },
 
   clearAll: function clear() {
@@ -223,63 +226,49 @@ this.PushDB.prototype = {
         },
         resolve,
         reject
       )
     );
   },
 
   /**
-   * Updates all push registrations for an origin.
+   * Reduces all records associated with an origin to a single value.
    *
    * @param {String} origin The origin, matched as a prefix against the scope.
    * @param {String} originAttributes Additional origin attributes. Requires
    *  an exact match.
-   * @param {Function} updateFunc A function that receives the existing
-   *  registration record as its argument, and returns a new record. The record
-   *  will not be updated if the function returns `null`, `undefined`, or an
-   *  invalid record. If the function returns `false`, the record will be
-   *  dropped.
-   * @returns {Promise} A promise that resolves once all records have been
-   *  updated.
+   * @param {Function} callback A function with the signature `(result,
+   *  record, cursor)`, where `result` is the value returned by the previous
+   *  invocation, `record` is the registration, and `cursor` is an `IDBCursor`.
+   * @param {Object} [initialValue] The value to use for the first invocation.
+   * @returns {Promise} Resolves with the value of the last invocation.
    */
-  updateByOrigin: function(origin, originAttributes, updateFunc) {
-    debug("updateByOrigin()");
+  reduceByOrigin: function(origin, originAttributes, callback, initialValue) {
+    debug("forEachOrigin()");
 
     return new Promise((resolve, reject) =>
       this.newTxn(
         "readwrite",
         this._dbStoreName,
         (aTxn, aStore) => {
-          aTxn.result = [];
+          aTxn.result = initialValue;
 
           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) {
               return;
             }
             let record = this.toPushRecord(cursor.value);
-            let newRecord = updateFunc(record);
-            if (newRecord === false) {
-              debug("updateByOrigin: Removing record for key ID " +
-                record.keyID);
-              cursor.delete();
-            } else if (this.isValidRecord(newRecord)) {
-              debug("updateByOrigin: Updating record for key ID " +
-                record.keyID);
-              cursor.update(newRecord);
-            } else {
-              debug("updateByOrigin: Ignoring invalid update for key ID " +
-                record.keyID + ": " + JSON.stringify(newRecord));
-            }
+            aTxn.result = callback(aTxn.result, record, cursor);
             cursor.continue();
           };
         },
         resolve,
         reject
       )
     );
   },
--- a/dom/push/PushRecord.jsm
+++ b/dom/push/PushRecord.jsm
@@ -189,43 +189,39 @@ PushRecord.prototype = {
    */
   hasPermission() {
     let permission = Services.perms.testExactPermissionFromPrincipal(
       this.principal, "desktop-notification");
     return permission == Ci.nsIPermissionManager.ALLOW_ACTION;
   },
 
   quotaChanged() {
+    if (!this.hasPermission()) {
+      return Promise.resolve(false);
+    }
     return this.getLastVisit()
       .then(lastVisit => lastVisit > this.lastPush);
   },
 
   quotaApplies() {
     return Number.isFinite(this.quota);
   },
 
   isExpired() {
     return this.quota === 0;
   },
 
-  toRegistration() {
+  toSubscription() {
     return {
       pushEndpoint: this.pushEndpoint,
       lastPush: this.lastPush,
       pushCount: this.pushCount,
       p256dhKey: this.p256dhPublicKey,
     };
   },
-
-  toRegister() {
-    return {
-      pushEndpoint: this.pushEndpoint,
-      p256dhKey: this.p256dhPublicKey,
-    };
-  },
 };
 
 // Define lazy getters for the principal and scope URI. IndexedDB can't store
 // `nsIPrincipal` objects, so we keep them in a private weak map.
 var principals = new WeakMap();
 Object.defineProperties(PushRecord.prototype, {
   principal: {
     get() {
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -290,41 +290,48 @@ this.PushService = {
           return;
         }
 
         var originAttributes =
           ChromeUtils.originAttributesToSuffix({ appId: data.appId,
                                                  inBrowser: data.browserOnly });
         this._db.getAllByOriginAttributes(originAttributes)
           .then(records => Promise.all(records.map(record =>
-            this._db.delete(record.keyID).then(
-              _ => this._unregisterIfConnected(record),
-              err => {
+            this._db.delete(record.keyID)
+              .catch(err => {
                 debug("webapps-clear-data: " + record.scope +
-                      " Could not delete entry " + record.channelID);
-
-                return this._unregisterIfConnected(record);
+                      " Could not delete entry " + record.keyID);
+                // This is the record we were unable to delete.
+                return record;
               })
+              .then(maybeDeleted => this._backgroundUnregister(maybeDeleted))
             )
           ));
 
         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);
-          });
+  /**
+   * Sends an unregister request to the server in the background. If the
+   * service is not connected, this function is a no-op.
+   *
+   * @param {PushRecord} record The record to unregister.
+   */
+  _backgroundUnregister: function(record) {
+    debug("backgroundUnregister()");
+
+    if (!this._service.isConnected() || !record) {
+      return;
     }
+
+    debug("Had a connection, so telling the server");
+    this._sendUnregister(record).catch(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";
@@ -584,17 +591,17 @@ this.PushService = {
     }
     if (event == UNINIT_EVENT) {
       // If it is uninitialized just close db.
       this._db.close();
       this._db = null;
       return Promise.resolve();
     }
 
-    return this.dropRegistrations()
+    return this.dropUnexpiredRegistrations()
        .then(_ => {
          this._db.close();
          this._db = null;
        }, err => {
          this._db.close();
          this._db = null;
        });
   },
@@ -675,22 +682,45 @@ this.PushService = {
   stopAlarm: function() {
     if (this._alarmID !== null) {
       debug("Stopped existing alarm " + this._alarmID);
       AlarmService.remove(this._alarmID);
       this._alarmID = null;
     }
   },
 
-  dropRegistrations: function() {
-    return this._notifyAllAppsRegister()
-      .then(_ => this._db.drop());
+  /**
+   * Drops all active registrations and notifies the associated service
+   * workers. This function is called when the user switches Push servers,
+   * or when the server invalidates all existing registrations.
+   *
+   * We ignore expired registrations because they're already handled in other
+   * code paths. Registrations that expired after exceeding their quotas are
+   * evicted at startup, or on the next `idle-daily` event. Registrations that
+   * expired because the user revoked the notification permission are evicted
+   * once the permission is reinstated.
+   */
+  dropUnexpiredRegistrations: function() {
+    let subscriptionChanges = [];
+    return this._db.clearIf(record => {
+      if (record.isExpired()) {
+        return false;
+      }
+      subscriptionChanges.push(record);
+      return true;
+    }).then(() => {
+      this.notifySubscriptionChanges(subscriptionChanges);
+    });
   },
 
   _notifySubscriptionChangeObservers: function(record) {
+    if (!record) {
+      return;
+    }
+
     // Notify XPCOM observers.
     Services.obs.notifyObservers(
       null,
       "push-subscription-change",
       record.scope
     );
 
     let data = {
@@ -715,39 +745,59 @@ this.PushService = {
       }
     } else {
       let ppmm = Cc['@mozilla.org/parentprocessmessagemanager;1']
                    .getService(Ci.nsIMessageListenerManager);
       ppmm.broadcastAsyncMessage(name, data);
     }
   },
 
-  // Fires a push-register system message to all applications that have
-  // registration.
-  _notifyAllAppsRegister: function() {
-    debug("notifyAllAppsRegister()");
-    // records are objects describing the registration as stored in IndexedDB.
-    return this._db.getAllUnexpired().then(records => {
-      records.forEach(record => {
-        this._notifySubscriptionChangeObservers(record);
-      });
-    });
+  /**
+   * Drops a registration and notifies the associated service worker. If the
+   * registration does not exist, this function is a no-op.
+   *
+   * @param {String} keyID The registration ID to remove.
+   * @returns {Promise} Resolves once the worker has been notified.
+   */
+  dropRegistrationAndNotifyApp: function(aKeyID) {
+    return this._db.delete(aKeyID)
+      .then(record => this._notifySubscriptionChangeObservers(record));
   },
 
-  dropRegistrationAndNotifyApp: function(aKeyId) {
-    return this._db.getByKeyID(aKeyId).then(record => {
-      this._notifySubscriptionChangeObservers(record);
-      return this._db.delete(aKeyId);
-    });
+  /**
+   * Replaces an existing registration and notifies the associated service
+   * worker.
+   *
+   * @param {String} aOldKey The registration ID to replace.
+   * @param {PushRecord} aNewRecord The new record.
+   * @returns {Promise} Resolves once the worker has been notified.
+   */
+  updateRegistrationAndNotifyApp: function(aOldKey, aNewRecord) {
+    return this.updateRecordAndNotifyApp(aOldKey, _ => aNewRecord);
+  },
+  /**
+   * Updates a registration and notifies the associated service worker.
+   *
+   * @param {String} keyID The registration ID to update.
+   * @param {Function} updateFunc Returns the updated record.
+   * @returns {Promise} Resolves with the updated record once the worker
+   *  has been notified.
+   */
+  updateRecordAndNotifyApp: function(aKeyID, aUpdateFunc) {
+    return this._db.update(aKeyID, aUpdateFunc)
+      .then(record => {
+        this._notifySubscriptionChangeObservers(record);
+        return record;
+      });
   },
 
-  updateRegistrationAndNotifyApp: function(aOldKey, aRecord) {
-    return this._db.delete(aOldKey)
-      .then(_ => this._db.put(aRecord))
-      .then(record => this._notifySubscriptionChangeObservers(record));
+  notifySubscriptionChanges: function(records) {
+    records.forEach(record => {
+      this._notifySubscriptionChangeObservers(record);
+    });
   },
 
   ensureP256dhKey: function(record) {
     if (record.p256dhPublicKey && record.p256dhPrivateKey) {
       return Promise.resolve(record);
     }
     // We do not have a encryption key. so we need to generate it. This
     // is only going to happen on db upgrade from version 4 to higher.
@@ -759,29 +809,16 @@ this.PushService = {
           return record;
         });
       }, error => {
         return this.dropRegistrationAndNotifyApp(record.keyID).then(
           () => Promise.reject(error));
       });
   },
 
-  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
@@ -1029,17 +1066,17 @@ this.PushService = {
         if (!record) {
           return this._lookupOrPutPendingRequest(aPageRecord);
         }
         if (record.isExpired()) {
           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);
+              return this.dropRegistrationAndNotifyApp(record.keyID);
             }
             throw {state: 0, error: "NotFoundError"};
           }).then(_ => this._lookupOrPutPendingRequest(aPageRecord));
         }
         return record;
       }, error => {
         debug("getByIdentifiers failed");
         throw error;
@@ -1168,17 +1205,17 @@ this.PushService = {
     this[aMessage.name.slice("Push:".length).toLowerCase()](pageRecord, mm);
   },
 
   register: function(aPageRecord, aMessageManager) {
     debug("register(): " + JSON.stringify(aPageRecord));
 
     this._register(aPageRecord)
       .then(record => {
-        let message = record.toRegister();
+        let message = record.toSubscription();
         message.requestID = aPageRecord.requestID;
         aMessageManager.sendAsyncMessage("PushService:Register:OK", message);
       }, error => {
         let message = {
           requestID: aPageRecord.requestID,
           error
         };
         aMessageManager.sendAsyncMessage("PushService:Register:KO", message);
@@ -1309,22 +1346,22 @@ this.PushService = {
       .then(_ => this._db.getByIdentifiers(aPageRecord))
       .then(record => {
         if (!record) {
           return null;
         }
         if (record.isExpired()) {
           return record.quotaChanged().then(isChanged => {
             if (isChanged) {
-              return this._db.delete(record.keyID).then(_ => null);
+              return this.dropRegistrationAndNotifyApp(record.keyID).then(_ => null);
             }
             return null;
           });
         }
-        return record.toRegistration();
+        return record.toSubscription();
       });
   },
 
   registration: function(aPageRecord, aMessageManager) {
     debug("registration()");
 
     return this._registration(aPageRecord)
       .then(registration =>
@@ -1343,17 +1380,17 @@ this.PushService = {
     debug("dropExpiredRegistrations()");
 
     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.dropRecordAndNotifyApp(record);
+            return this.dropRegistrationAndNotifyApp(record.keyID);
           }
         }).catch(error => {
           debug("dropExpiredRegistrations: Error dropping registration " +
             record.keyID + ": " + error);
         })
       ));
     });
   },
@@ -1363,17 +1400,17 @@ this.PushService = {
 
     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);
+            this._backgroundUnregister(record);
           }
           return true;
         }
         return false;
       });
     }
 
     let permission = subject.QueryInterface(Ci.nsIPermission);
@@ -1390,74 +1427,89 @@ this.PushService = {
     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._updateByPrincipal(
+      return this._reduceByPrincipal(
         permission.principal,
-        record => this._permissionAllowed(record)
-      );
+        (subscriptionChanges, record, cursor) => {
+          this._permissionAllowed(subscriptionChanges, record, cursor);
+          return subscriptionChanges;
+        },
+        []
+      ).then(subscriptionChanges => {
+        this.notifySubscriptionChanges(subscriptionChanges);
+      });
     } 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._updateByPrincipal(
+      return this._reduceByPrincipal(
         permission.principal,
-        record => this._permissionDenied(record)
+        (memo, record, cursor) => this._permissionDenied(record, cursor)
       );
     }
 
     return Promise.resolve();
   },
 
-  _updateByPrincipal: function(principal, updateFunc) {
-    return this._db.updateByOrigin(
+  _reduceByPrincipal: function(principal, callback, initialValue) {
+    return this._db.reduceByOrigin(
       principal.URI.prePath,
       ChromeUtils.originAttributesToSuffix(principal.originAttributes),
-      updateFunc
+      callback,
+      initialValue
     );
   },
 
   /**
-   * 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.
+   * The update function called for each registration record if the push
+   * permission is revoked. We only expire the record so we can notify the
+   * service worker as soon as the permission is reinstated. If we just
+   * deleted the record, 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.
+   * @param {PushRecord} record The record to expire.
+   * @param {IDBCursor} cursor The IndexedDB cursor.
    */
-  _permissionDenied: function(record) {
+  _permissionDenied: function(record, cursor) {
+    debug("permissionDenied()");
+
     if (!record.quotaApplies() || record.isExpired()) {
       // Ignore already-expired records.
-      return null;
+      return;
     }
     // Drop the registration in the background.
-    this._unregisterIfConnected(record);
+    this._backgroundUnregister(record);
     record.setQuota(0);
-    return record;
+    cursor.update(record);
   },
 
   /**
-   * Drops all expired registrations, notifies the associated service
-   * workers, and resets the quota for active registrations if the push
-   * permission is granted.
+   * The update function called for each registration record if the push
+   * permission is granted. If the record has expired, it will be dropped;
+   * otherwise, its quota will be reset to the default value.
    *
-   * @param {Array} A list of records to refresh.
-   * @returns {Promise} A promise resolved with the refreshed records.
+   * @param {Array} subscriptionChanges A list of records whose associated
+   *  service workers should be notified once the transaction has committed.
+   * @param {PushRecord} record The record to update.
+   * @param {IDBCursor} cursor The IndexedDB cursor.
    */
-  _permissionAllowed: function(record) {
+  _permissionAllowed: function(subscriptionChanges, record, cursor) {
+    debug("permissionAllowed()");
+
     if (!record.quotaApplies()) {
-      return null;
+      return;
     }
     if (record.isExpired()) {
       // If the registration has expired, drop and notify the worker
       // unconditionally.
-      this._notifySubscriptionChangeObservers(record);
-      return false;
+      subscriptionChanges.push(record);
+      cursor.delete();
+      return;
     }
     record.resetQuota();
-    return record;
+    cursor.update(record);
   },
 };
--- a/dom/push/PushServiceHttp2.jsm
+++ b/dom/push/PushServiceHttp2.jsm
@@ -873,19 +873,13 @@ function PushRecordHttp2(record) {
 PushRecordHttp2.prototype = Object.create(PushRecord.prototype, {
   keyID: {
     get() {
       return this.subscriptionUri;
     },
   },
 });
 
-PushRecordHttp2.prototype.toRegistration = function() {
-  let registration = PushRecord.prototype.toRegistration.call(this);
-  registration.pushReceiptEndpoint = this.pushReceiptEndpoint;
-  return registration;
+PushRecordHttp2.prototype.toSubscription = function() {
+  let subscription = PushRecord.prototype.toSubscription.call(this);
+  subscription.pushReceiptEndpoint = this.pushReceiptEndpoint;
+  return subscription;
 };
-
-PushRecordHttp2.prototype.toRegister = function() {
-  let register = PushRecord.prototype.toRegister.call(this);
-  register.pushReceiptEndpoint = this.pushReceiptEndpoint;
-  return register;
-};
--- a/dom/push/PushServiceWebSocket.jsm
+++ b/dom/push/PushServiceWebSocket.jsm
@@ -836,17 +836,17 @@ this.PushServiceWebSocket = {
     // accept.
     //
     // We unconditionally drop all existing registrations and notify service
     // workers if we receive a new UAID. This ensures we expunge all stale
     // registrations if the `userAgentID` pref is reset.
     if (this._UAID != reply.uaid) {
       debug("got new UAID: all re-register");
 
-      this._mainPushService.dropRegistrations()
+      this._mainPushService.dropUnexpiredRegistrations()
           .then(finishHandshake.bind(this));
 
       return;
     }
 
     // otherwise we are good to go
     finishHandshake.bind(this)();
   },
@@ -1461,13 +1461,13 @@ function PushRecordWebSocket(record) {
 PushRecordWebSocket.prototype = Object.create(PushRecord.prototype, {
   keyID: {
     get() {
       return this.channelID;
     },
   },
 });
 
-PushRecordWebSocket.prototype.toRegistration = function() {
-  let registration = PushRecord.prototype.toRegistration.call(this);
-  registration.version = this.version;
-  return registration;
+PushRecordWebSocket.prototype.toSubscription = function() {
+  let subscription = PushRecord.prototype.toSubscription.call(this);
+  subscription.version = this.version;
+  return subscription;
 };
--- a/dom/push/test/xpcshell/head.js
+++ b/dom/push/test/xpcshell/head.js
@@ -2,16 +2,17 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 'use strict';
 
 var {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import('resource://gre/modules/XPCOMUtils.jsm');
 Cu.import('resource://gre/modules/Services.jsm');
+Cu.import('resource://gre/modules/Task.jsm');
 Cu.import('resource://gre/modules/Timer.jsm');
 Cu.import('resource://gre/modules/Promise.jsm');
 Cu.import('resource://gre/modules/Preferences.jsm');
 Cu.import('resource://gre/modules/PlacesUtils.jsm');
 
 const serviceExports = Cu.import('resource://gre/modules/PushService.jsm', {});
 const servicePrefs = new Preferences('dom.push.');
 
new file mode 100644
--- /dev/null
+++ b/dom/push/test/xpcshell/test_drop_expired.js
@@ -0,0 +1,153 @@
+/* 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';
+
+var db;
+var quotaURI;
+var permURI;
+
+function visitURI(uri, timestamp) {
+  return addVisit({
+    uri: uri,
+    title: uri.spec,
+    visits: [{
+      visitDate: timestamp * 1000,
+      transitionType: Ci.nsINavHistoryService.TRANSITION_LINK,
+    }],
+  });
+}
+
+var putRecord = Task.async(function* ({scope, perm, quota, lastPush, lastVisit}) {
+  let uri = Services.io.newURI(scope, null, null);
+
+  Services.perms.add(uri, 'desktop-notification',
+    Ci.nsIPermissionManager[perm]);
+  do_register_cleanup(() => {
+    Services.perms.remove(uri, 'desktop-notification');
+  });
+
+  yield visitURI(uri, lastVisit);
+
+  yield db.put({
+    channelID: uri.path,
+    pushEndpoint: 'https://example.org/push' + uri.path,
+    scope: uri.spec,
+    pushCount: 0,
+    lastPush: lastPush,
+    version: null,
+    originAttributes: '',
+    quota: quota,
+  });
+
+  return uri;
+});
+
+function run_test() {
+  do_get_profile();
+  setPrefs({
+    userAgentID: userAgentID,
+  });
+
+  db = PushServiceWebSocket.newPushDB();
+  do_register_cleanup(() => {return db.drop().then(_ => db.close());});
+
+  run_next_test();
+}
+
+add_task(function* setUp() {
+  // An expired registration that should be evicted on startup. Permission is
+  // granted for this origin, and the last visit is more recent than the last
+  // push message.
+  yield putRecord({
+    scope: 'https://example.com/expired-quota-restored',
+    perm: 'ALLOW_ACTION',
+    quota: 0,
+    lastPush: Date.now() - 10,
+    lastVisit: Date.now(),
+  });
+
+  // An expired registration that we should evict when the origin is visited
+  // again.
+  quotaURI = yield putRecord({
+    scope: 'https://example.xyz/expired-quota-exceeded',
+    perm: 'ALLOW_ACTION',
+    quota: 0,
+    lastPush: Date.now() - 10,
+    lastVisit: Date.now() - 20,
+  });
+
+  // An expired registration that we should evict when permission is granted
+  // again.
+  permURI = yield putRecord({
+    scope: 'https://example.info/expired-perm-revoked',
+    perm: 'DENY_ACTION',
+    quota: 0,
+    lastPush: Date.now() - 10,
+    lastVisit: Date.now(),
+  });
+
+  // An active registration that we should leave alone.
+  yield putRecord({
+    scope: 'https://example.ninja/active',
+    perm: 'ALLOW_ACTION',
+    quota: 16,
+    lastPush: Date.now() - 10,
+    lastVisit: Date.now() - 20,
+  });
+
+  let subChangePromise = promiseObserverNotification(
+    'push-subscription-change',
+    (subject, data) => data == 'https://example.com/expired-quota-restored'
+  );
+
+  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,
+          }));
+        },
+      });
+    },
+  });
+
+  yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for subscription change event on startup');
+});
+
+add_task(function* test_site_visited() {
+  let subChangePromise = promiseObserverNotification(
+    'push-subscription-change',
+    (subject, data) => data == 'https://example.xyz/expired-quota-exceeded'
+  );
+
+  yield visitURI(quotaURI, Date.now());
+  PushService.observe(null, 'idle-daily', '');
+
+  yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for subscription change event after visit');
+});
+
+add_task(function* test_perm_restored() {
+  let subChangePromise = promiseObserverNotification(
+    'push-subscription-change',
+    (subject, data) => data == 'https://example.info/expired-perm-revoked'
+  );
+
+  Services.perms.add(permURI, 'desktop-notification',
+    Ci.nsIPermissionManager.ALLOW_ACTION);
+
+  yield waitForPromise(subChangePromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for subscription change event after permission');
+});
--- a/dom/push/test/xpcshell/test_notification_incomplete.js
+++ b/dom/push/test/xpcshell/test_notification_incomplete.js
@@ -1,18 +1,22 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 'use strict';
 
 const {PushDB, PushService, PushServiceWebSocket} = serviceExports;
 
+const userAgentID = '1ca1cf66-eeb4-4df7-87c1-d5c92906ab90';
+
 function run_test() {
   do_get_profile();
-  setPrefs();
+  setPrefs({
+    userAgentID: userAgentID,
+  });
   disableServiceWorkerEvents(
     'https://example.com/page/1',
     'https://example.com/page/2',
     'https://example.com/page/3',
     'https://example.com/page/4'
   );
   run_next_test();
 }
@@ -69,17 +73,17 @@ add_task(function* test_notification_inc
     networkInfo: new MockDesktopNetworkInfo(),
     db,
     makeWebSocket(uri) {
       return new MockWebSocket(uri, {
         onHello(request) {
           this.serverSendMsg(JSON.stringify({
             messageType: 'hello',
             status: 200,
-            uaid: '1ca1cf66-eeb4-4df7-87c1-d5c92906ab90'
+            uaid: userAgentID,
           }));
           this.serverSendMsg(JSON.stringify({
             // Missing "updates" field; should ignore message.
             messageType: 'notification'
           }));
           this.serverSendMsg(JSON.stringify({
             messageType: 'notification',
             updates: [{
--- a/dom/push/test/xpcshell/test_quota_observer.js
+++ b/dom/push/test/xpcshell/test_quota_observer.js
@@ -2,42 +2,56 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 'use strict';
 
 const {PushDB, PushService, PushServiceWebSocket} = serviceExports;
 
 const userAgentID = '28cd09e2-7506-42d8-9e50-b02785adc7ef';
 
+var db;
+
 function run_test() {
   do_get_profile();
   setPrefs({
     userAgentID,
   });
   run_next_test();
 }
 
+let putRecord = Task.async(function* (perm, record) {
+  let uri = Services.io.newURI(record.scope, null, null);
+
+  Services.perms.add(uri, 'desktop-notification',
+    Ci.nsIPermissionManager[perm]);
+  do_register_cleanup(() => {
+    Services.perms.remove(uri, 'desktop-notification');
+  });
+
+  yield db.put(record);
+});
+
 add_task(function* test_expiration_history_observer() {
-  let db = PushServiceWebSocket.newPushDB();
+  db = PushServiceWebSocket.newPushDB();
   do_register_cleanup(() => db.drop().then(_ => db.close()));
 
   // A registration that we'll expire...
-  yield db.put({
+  yield putRecord('ALLOW_ACTION', {
     channelID: '379c0668-8323-44d2-a315-4ee83f1a9ee9',
     pushEndpoint: 'https://example.org/push/1',
     scope: 'https://example.com/deals',
     pushCount: 0,
     lastPush: 0,
     version: null,
     originAttributes: '',
     quota: 16,
   });
 
   // ...And a registration that we'll evict on startup.
-  yield db.put({
+  yield putRecord('ALLOW_ACTION', {
     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,
@@ -96,17 +110,17 @@ add_task(function* test_expiration_histo
 
   let notifiedScopes = [];
   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({
+  yield putRecord('ALLOW_ACTION', {
     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,
--- a/dom/push/test/xpcshell/xpcshell.ini
+++ b/dom/push/test/xpcshell/xpcshell.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 head = head.js head-http2.js
 tail =
 # Push notifications and alarms are currently disabled on Android.
 skip-if = toolkit == 'android'
 
+[test_drop_expired.js]
 [test_notification_ack.js]
 [test_notification_data.js]
 [test_notification_duplicate.js]
 [test_notification_error.js]
 [test_notification_incomplete.js]
 [test_notification_version_string.js]
 
 [test_permissions.js]