Bug 1210211 - Part 1: Delay updating push quota. r=kitcambridge a=ritu
authorWilliam Chen <wchen@mozilla.com>
Mon, 16 Nov 2015 21:33:11 -0800
changeset 284916 2e2780789509cd4b281bffc004dff635d4930eca
parent 284915 6c630e37e53f15976e1bdad36852e74fd073f3f7
child 284917 d7a61804d40ee969ab258a8509f87e184b6a7d11
push id198
push usercbook@mozilla.com
push dateWed, 02 Dec 2015 07:27:51 +0000
reviewerskitcambridge, ritu
bugs1210211
milestone44.0a2
Bug 1210211 - Part 1: Delay updating push quota. r=kitcambridge a=ritu
dom/push/PushRecord.jsm
dom/push/PushService.jsm
dom/push/test/xpcshell/head.js
modules/libpref/init/all.js
--- a/dom/push/PushRecord.jsm
+++ b/dom/push/PushRecord.jsm
@@ -60,44 +60,43 @@ PushRecord.prototype = {
       return;
     }
     if (lastVisit < 0) {
       // If the user cleared their history, but retained the push permission,
       // mark the registration as expired.
       this.quota = 0;
       return;
     }
-    let currentQuota;
     if (lastVisit > this.lastPush) {
       // If the user visited the site since the last time we received a
       // notification, reset the quota.
       let daysElapsed = (Date.now() - lastVisit) / 24 / 60 / 60 / 1000;
-      currentQuota = Math.min(
+      this.quota = Math.min(
         Math.round(8 * Math.pow(daysElapsed, -0.8)),
         prefs.get("maxQuotaPerSubscription")
       );
-      Services.telemetry.getHistogramById("PUSH_API_QUOTA_RESET_TO").add(currentQuota - 1);
-    } else {
-      // The user hasn't visited the site since the last notification.
-      currentQuota = this.quota;
-    }
-    this.quota = Math.max(currentQuota - 1, 0);
-    // We check for ctime > 0 to skip older records that did not have ctime.
-    if (this.isExpired() && this.ctime > 0) {
-      let duration = Date.now() - this.ctime;
-      Services.telemetry.getHistogramById("PUSH_API_QUOTA_EXPIRATION_TIME").add(duration / 1000);
+      Services.telemetry.getHistogramById("PUSH_API_QUOTA_RESET_TO").add(this.quota);
     }
   },
 
   receivedPush(lastVisit) {
     this.updateQuota(lastVisit);
     this.pushCount++;
     this.lastPush = Date.now();
   },
 
+  reduceQuota() {
+    this.quota = Math.max(this.quota - 1, 0);
+    // We check for ctime > 0 to skip older records that did not have ctime.
+    if (this.isExpired() && this.ctime > 0) {
+      let duration = Date.now() - this.ctime;
+      Services.telemetry.getHistogramById("PUSH_API_QUOTA_EXPIRATION_TIME").add(duration / 1000);
+    }
+  },
+
   /**
    * Queries the Places database for the last time a user visited the site
    * associated with a push registration.
    *
    * @returns {Promise} A promise resolved with either the last time the user
    *  visited the site, or `-Infinity` if the site is not in the user's history.
    *  The time is expressed in milliseconds since Epoch.
    */
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -39,16 +39,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 this.EXPORTED_SYMBOLS = ["PushService"];
 
 const prefs = new Preferences("dom.push.");
 // Set debug first so that all debugging actually works.
 gDebuggingEnabled = prefs.get("debug");
 
 const kCHILD_PROCESS_MESSAGES = ["Push:Register", "Push:Unregister",
                                  "Push:Registration", "Push:RegisterEventNotificationListener",
+                                 "Push:NotificationForOriginShown",
+                                 "Push:NotificationForOriginClosed",
                                  "child-process-shutdown"];
 
 const PUSH_SERVICE_UNINIT = 0;
 const PUSH_SERVICE_INIT = 1; // No serverURI
 const PUSH_SERVICE_ACTIVATING = 2;//activating db
 const PUSH_SERVICE_CONNECTION_DISABLE = 3;
 const PUSH_SERVICE_ACTIVE_OFFLINE = 4;
 const PUSH_SERVICE_RUNNING = 5;
@@ -91,16 +93,21 @@ const UNINIT_EVENT = 3;
  * for persistence.
  */
 this.PushService = {
   _service: null,
   _state: PUSH_SERVICE_UNINIT,
   _db: null,
   _options: null,
   _alarmID: null,
+  _visibleNotifications: new Map(),
+
+  // Callback that is called after attempting to
+  // reduce the quota for a record. Used for testing purposes.
+  _updateQuotaTestCallback: null,
 
   _childListeners: [],
 
   // When serverURI changes (this is used for testing), db is cleaned up and a
   // a new db is started. This events must be sequential.
   _stateChangeProcessQueue: null,
   _stateChangeProcessEnqueue: function(op) {
     if (!this._stateChangeProcessQueue) {
@@ -842,32 +849,89 @@ this.PushService = {
         );
       } else {
         decodedPromise = Promise.resolve(null);
       }
       return decodedPromise.then(message => {
         if (shouldNotify) {
           notified = this._notifyApp(record, message);
         }
-        if (record.isExpired()) {
-          this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_EXPIRED);
-          // Drop the registration in the background. If the user returns to the
-          // site, the service worker will be notified on the next `idle-daily`
-          // event.
-          this._sendUnregister(record).catch(error => {
-            debug("receivedPushMessage: Unregister error: " + error);
-          });
-        }
+        // Update quota after the delay, at which point
+        // we check for visible notifications.
+        setTimeout(() => this._updateQuota(keyID),
+          prefs.get("quotaUpdateDelay"));
         return notified;
       });
     }).catch(error => {
       debug("receivedPushMessage: Error notifying app: " + error);
     });
   },
 
+  _updateQuota: function(keyID) {
+    debug("updateQuota()");
+
+    this._db.update(keyID, record => {
+      // Record may have expired from an earlier quota update.
+      if (record.isExpired()) {
+        debug("updateQuota: Trying to update quota for expired record " +
+          JSON.stringify(record));
+        return null;
+      }
+      // If there are visible notifications, don't apply the quota penalty
+      // for the message.
+      if (!this._visibleNotifications.has(record.uri.prePath)) {
+        record.reduceQuota();
+      }
+      return record;
+    }).then(record => {
+      if (record && record.isExpired()) {
+        this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_EXPIRED);
+        // Drop the registration in the background. If the user returns to the
+        // site, the service worker will be notified on the next `idle-daily`
+        // event.
+        this._sendUnregister(record).catch(error => {
+          debug("updateQuota: Unregister error: " + error);
+        });
+      }
+      if (this._updateQuotaTestCallback) {
+        // Callback so that test may be notified when the quota update is complete.
+        this._updateQuotaTestCallback();
+      }
+    }).catch(error => {
+      debug("updateQuota: Error while trying to update quota " + error);
+    });
+  },
+
+  _notificationForOriginShown(origin) {
+    debug("notificationForOriginShown() " + origin);
+    let count;
+    if (this._visibleNotifications.has(origin)) {
+      count = this._visibleNotifications.get(origin);
+    } else {
+      count = 0;
+    }
+    this._visibleNotifications.set(origin, count + 1);
+  },
+
+  _notificationForOriginClosed(origin) {
+    debug("notificationForOriginClosed() " + origin);
+    let count;
+    if (this._visibleNotifications.has(origin)) {
+      count = this._visibleNotifications.get(origin);
+    } else {
+      debug("notificationForOriginClosed: closing notification that has not been shown?");
+      return;
+    }
+    if (count > 1) {
+      this._visibleNotifications.set(origin, count - 1);
+    } else {
+      this._visibleNotifications.delete(origin);
+    }
+  },
+
   _notifyApp: function(aPushRecord, message) {
     if (!aPushRecord || !aPushRecord.scope ||
         aPushRecord.originAttributes === undefined) {
       debug("notifyApp() something is undefined.  Dropping notification: " +
         JSON.stringify(aPushRecord) );
       return false;
     }
 
@@ -1047,16 +1111,30 @@ this.PushService = {
     if (aMessage.name === "child-process-shutdown") {
       debug("Possibly removing child listener");
       for (var i = this._childListeners.length - 1; i >= 0; --i) {
         if (this._childListeners[i] == aMessage.target) {
           debug("Removed child listener");
           this._childListeners.splice(i, 1);
         }
       }
+      debug("Clearing notifications from child");
+      this._visibleNotifications.clear();
+      return;
+    }
+
+    if (aMessage.name === "Push:NotificationForOriginShown") {
+      debug("Notification shown from child");
+      this._notificationForOriginShown(aMessage.data);
+      return;
+    }
+
+    if (aMessage.name === "Push:NotificationForOriginClosed") {
+      debug("Notification closed from child");
+      this._notificationForOriginClosed(aMessage.data);
       return;
     }
 
     if (!aMessage.target.assertPermission("push")) {
       debug("Got message from a child process that does not have 'push' permission.");
       return null;
     }
 
--- a/dom/push/test/xpcshell/head.js
+++ b/dom/push/test/xpcshell/head.js
@@ -216,16 +216,17 @@ function setPrefs(prefs = {}) {
     'adaptive.gap': 60000,
     'adaptive.upperLimit': 29 * 60 * 1000,
     // Misc. defaults.
     'adaptive.mobile': '',
     'http2.maxRetries': 2,
     'http2.retryInterval': 500,
     'http2.reset_retry_count_after_ms': 60000,
     maxQuotaPerSubscription: 16,
+    quotaUpdateDelay: 3000,
   }, prefs);
   for (let pref in defaultPrefs) {
     servicePrefs.set(pref, defaultPrefs[pref]);
   }
 }
 
 function compareAscending(a, b) {
   return a > b ? 1 : a < b ? -1 : 0;
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4469,20 +4469,24 @@ pref("dom.mozAlarms.enabled", false);
 
 pref("dom.push.enabled", false);
 
 pref("dom.push.debug", false);
 
 pref("dom.push.serverURL", "wss://push.services.mozilla.com/");
 pref("dom.push.userAgentID", "");
 
-// The maximum number of notifications that a service worker can receive
+// The maximum number of push messages that a service worker can receive
 // without user interaction.
 pref("dom.push.maxQuotaPerSubscription", 16);
 
+// The delay between receiving a push message and updating the quota for a
+// subscription.
+pref("dom.push.quotaUpdateDelay", 3000); // 3 seconds
+
 // Is the network connection allowed to be up?
 // This preference should be used in UX to enable/disable push.
 pref("dom.push.connection.enabled", true);
 
 // Exponential back-off start is 5 seconds like in HTTP/1.1.
 // Maximum back-off is pingInterval.
 pref("dom.push.retryBaseInterval", 5000);