Bug 1223202 - Only send subscription change events if the Push permission is granted. r=mt
authorKit Cambridge <kcambridge@mozilla.com>
Tue, 10 Nov 2015 14:27:47 -0800
changeset 306348 7347ad233dd011ea1381fd516b7afe5f40bdce0a
parent 306347 f35d1107fe2eabc3128c9430724fa730c3336fd5
child 306349 5d8b8744419fb6d617d247d7b37fbe60a67a2b35
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt
bugs1223202
milestone45.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 1223202 - Only send subscription change events if the Push permission is granted. r=mt
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
@@ -122,17 +122,20 @@ this.PushDB.prototype = {
     console.debug("delete()");
 
     return new Promise((resolve, reject) =>
       this.newTxn(
         "readwrite",
         this._dbStoreName,
         function txnCb(aTxn, aStore) {
           console.debug("delete: Removing record", aKeyID);
-          aStore.delete(aKeyID);
+          aStore.get(aKeyID).onsuccess = event => {
+            aTxn.result = event.target.result;
+            aStore.delete(aKeyID);
+          };
         },
         resolve,
         reject
       )
     );
   },
 
   // testFn(record) is called with a database record and should return true if
@@ -208,63 +211,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) {
-    console.debug("updateByOrigin()");
+  reduceByOrigin: function(origin, originAttributes, callback, initialValue) {
+    console.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) {
-              console.debug("updateByOrigin: Removing record for key ID",
-                record.keyID);
-              cursor.delete();
-            } else if (this.isValidRecord(newRecord)) {
-              console.debug("updateByOrigin: Updating record for key ID",
-                record.keyID, newRecord);
-              cursor.update(newRecord);
-            } else {
-              console.error("updateByOrigin: Ignoring invalid update for record",
-                record.keyID, newRecord);
-            }
+            aTxn.result = callback(aTxn.result, record, cursor);
             cursor.continue();
           };
         },
         resolve,
         reject
       )
     );
   },
--- a/dom/push/PushRecord.jsm
+++ b/dom/push/PushRecord.jsm
@@ -190,43 +190,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
@@ -277,41 +277,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._backgroundUnregister(record),
-              err => {
+            this._db.delete(record.keyID)
+              .catch(err => {
                 console.error("webapps-clear-data: Error removing record",
                   record, err);
-
-                this._backgroundUnregister(record);
+                // This is the record we were unable to delete.
+                return record;
               })
+              .then(maybeDeleted => this._backgroundUnregister(maybeDeleted))
             )
           ));
 
         break;
     }
   },
 
+  /**
+   * 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) {
-    if (this._service.isConnected()) {
-      // courtesy, but don't establish a connection
-      // just for it
-      console.debug("backgroundUnregister: Notifying server", record);
-      return this._sendUnregister(record)
-          .catch(function(e) {
-            console.error("backgroundUnregister: Error notifying server", e);
-          });
+    console.debug("backgroundUnregister()");
+
+    if (!this._service.isConnected() || !record) {
+      return;
     }
+
+    console.debug("backgroundUnregister: Notifying server", record);
+    this._sendUnregister(record).catch(e => {
+      console.error("backgroundUnregister: Error notifying server", 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";
@@ -568,17 +575,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;
        });
   },
@@ -659,22 +666,45 @@ this.PushService = {
   stopAlarm: function() {
     if (this._alarmID !== null) {
       console.debug("stopAlarm: 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 = {
@@ -699,39 +729,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() {
-    console.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.
@@ -743,29 +793,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
@@ -930,17 +967,17 @@ this.PushService = {
     console.debug("registerWithServer()", aPageRecord);
 
     Services.telemetry.getHistogramById("PUSH_API_SUBSCRIBE_ATTEMPT").add();
     return this._sendRequest("register", aPageRecord)
       .then(record => this._onRegisterSuccess(record),
             err => this._onRegisterError(err))
       .then(record => {
         this._deletePendingRequest(aPageRecord);
-        return record.toRegister();
+        return record.toSubscription();
       }, err => {
         this._deletePendingRequest(aPageRecord);
         throw err;
      });
   },
 
   _sendUnregister: function(aRecord) {
     Services.telemetry.getHistogramById("PUSH_API_UNSUBSCRIBE_ATTEMPT").add();
@@ -1071,22 +1108,22 @@ 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 new Error("Push subscription expired");
           }).then(_ => this._lookupOrPutPendingRequest(aPageRecord));
         }
-        return record.toRegister();
+        return record.toSubscription();
       });
   },
 
   /**
    * Called on message from the child process.
    *
    * Why is the record being deleted from the local database before the server
    * is told?
@@ -1189,35 +1226,35 @@ 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();
       });
   },
 
   _dropExpiredRegistrations: function() {
     console.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 => {
           console.error("dropExpiredRegistrations: Error dropping registration",
             record.keyID, error);
         })
       ));
     });
   },
@@ -1254,74 +1291,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) {
+    console.debug("permissionDenied()");
+
     if (!record.quotaApplies() || record.isExpired()) {
       // Ignore already-expired records.
-      return null;
+      return;
     }
     // Drop the registration in the background.
     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) {
+    console.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
@@ -853,19 +853,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
@@ -835,17 +835,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) {
       console.debug("handleHelloReply: Received new UAID");
 
-      this._mainPushService.dropRegistrations()
+      this._mainPushService.dropUnexpiredRegistrations()
           .then(finishHandshake.bind(this));
 
       return;
     }
 
     // otherwise we are good to go
     finishHandshake.bind(this)();
   },
@@ -1467,13 +1467,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]