Bug 914481 - Patch 4 - Track why we fail to deliver notifications to Service Workers. r=kitcambridge
☠☠ backed out by b6e914c70e41 ☠ ☠
authorNikhil Marathe <nsm.nikhil@gmail.com>
Thu, 06 Aug 2015 16:59:35 -0700
changeset 295614 6165f6eba17f51cec7e79d83d9bc92ddf9c7162a
parent 295613 a8c47e9431d00712951c8d434a34120fe50322b3
child 295615 3560ae11e7eb40732be6f2b5e9c0b1eb69c84849
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs914481
milestone43.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 914481 - Patch 4 - Track why we fail to deliver notifications to Service Workers. r=kitcambridge
dom/push/PushService.jsm
toolkit/components/telemetry/Histograms.json
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -47,16 +47,26 @@ const kCHILD_PROCESS_MESSAGES = ["Push:R
 
 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;
 
+// Telemetry failure to send push notification to Service Worker reasons.
+// Key not found in local database.
+const kDROP_NOTIFICATION_REASON_KEY_NOT_FOUND = 0;
+// User cleared history.
+const kDROP_NOTIFICATION_REASON_NO_HISTORY = 1;
+// Version of message received not newer than previous one.
+const kDROP_NOTIFICATION_REASON_NO_VERSION_INCREMENT = 2;
+// Subscription has expired.
+const kDROP_NOTIFICATION_REASON_EXPIRED = 3;
+
 /**
  * State is change only in couple of functions:
  *   init - change state to PUSH_SERVICE_INIT if state was PUSH_SERVICE_UNINIT
  *   changeServerURL - change state to PUSH_SERVICE_ACTIVATING if serverURL
  *                     present or PUSH_SERVICE_INIT if not present.
  *   changeStateConnectionEnabledEvent - it is call on pref change or during
  *                                       the service activation and it can
  *                                       change state to
@@ -668,16 +678,17 @@ this.PushService = {
       record.scope
     );
 
     let data = {
       originAttributes: record.originAttributes,
       scope: record.scope
     };
 
+    Services.telemetry.getHistogramById("PUSH_API_NOTIFY_REGISTRATION_LOST").add();
     this._notifyListeners('pushsubscriptionchange', data);
   },
 
   _notifyListeners: function(name, data) {
     if (this._childListeners.length > 0) {
       // Try to send messages to all listeners, but remove any that fail since
       // the receiver is likely gone away.
       for (var i = this._childListeners.length - 1; i >= 0; --i) {
@@ -719,52 +730,66 @@ this.PushService = {
       .then(record => this._notifySubscriptionChangeObservers(record));
   },
 
   updateRecordAndNotifyApp: function(aKeyID, aUpdateFunc) {
     return this._db.update(aKeyID, aUpdateFunc)
       .then(record => this._notifySubscriptionChangeObservers(record));
   },
 
+  _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
    * quota for the associated push registration. If the quota is exceeded,
    * the registration and message will be dropped, and the worker will not
    * be notified.
    *
    * @param {String} keyID The push registration ID.
    * @param {String} message The message contents.
    * @param {Function} updateFunc A function that receives the existing
    *  registration record as its argument, and returns a new record. If the
    *  function returns `null` or `undefined`, the record will not be updated.
    *  `PushServiceWebSocket` uses this to drop incoming updates with older
    *  versions.
    */
   receivedPushMessage: function(keyID, message, updateFunc) {
     debug("receivedPushMessage()");
+    Services.telemetry.getHistogramById("PUSH_API_NOTIFICATION_RECEIVED").add();
 
     let shouldNotify = false;
     return this.getByKeyID(keyID).then(record => {
       if (!record) {
+        this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_KEY_NOT_FOUND);
         throw new Error("No record for key ID " + keyID);
       }
       return record.getLastVisit();
     }).then(lastVisit => {
       // As a special case, don't notify the service worker if the user
       // cleared their history.
       shouldNotify = isFinite(lastVisit);
+      if (!shouldNotify) {
+          this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_NO_HISTORY);
+      }
       return this._db.update(keyID, record => {
         let newRecord = updateFunc(record);
         if (!newRecord) {
+          this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_NO_VERSION_INCREMENT);
           return null;
         }
+        // FIXME(nsm): WHY IS expired checked here but then also checked in the next case?
         if (newRecord.isExpired()) {
           // Because `unregister` is advisory only, we can still receive messages
           // for stale registrations from the server.
           debug("receivedPushMessage: Ignoring update for expired key ID " + keyID);
+          this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_EXPIRED);
           return null;
         }
         newRecord.receivedPush(lastVisit);
         return newRecord;
       });
     }).then(record => {
       var notified = false;
       if (!record) {
@@ -820,16 +845,17 @@ this.PushService = {
 
     // TODO data.
     let data = {
       payload: message,
       originAttributes: aPushRecord.originAttributes,
       scope: aPushRecord.scope
     };
 
+    Services.telemetry.getHistogramById("PUSH_API_NOTIFY").add();
     this._notifyListeners('push', data);
     return true;
   },
 
   getByKeyID: function(aKeyID) {
     return this._db.getByKeyID(aKeyID);
   },
 
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -9426,25 +9426,50 @@
   },
   "PUSH_API_QUOTA_EXPIRATION_TIME": {
     "alert_emails": ["push@mozilla.com"],
     "expires_in_version": "55",
     "kind": "exponential",
     "high": "31622400",
     "n_buckets": 20,
     "description": "Time taken for a push subscription to expire its quota (seconds). The maximum is just over an year."
-  }
+  },
   "PUSH_API_QUOTA_RESET_TO": {
     "alert_emails": ["push@mozilla.com"],
     "expires_in_version": "55",
     "kind": "exponential",
     "high": "200",
     "n_buckets": 10,
     "description": "The value a push record quota (a count) is reset to based on the user's browsing history."
   },
+  "PUSH_API_NOTIFICATION_RECEIVED": {
+    "alert_emails": ["push@mozilla.com"],
+    "expires_in_version": "55",
+    "kind": "count",
+    "description": "Push notification was received from server."
+  },
+  "PUSH_API_NOTIFICATION_RECEIVED_BUT_DID_NOT_NOTIFY": {
+    "alert_emails": ["push@mozilla.com"],
+    "expires_in_version": "55",
+    "kind": "enumerated",
+    "n_values": 15,
+    "description": "Push notification was received from server, but not delivered to ServiceWorker. Enumeration values are defined in dom/push/PushService.jsm as kDROP_NOTIFICATION_REASON_*."
+  },
+  "PUSH_API_NOTIFY": {
+    "alert_emails": ["push@mozilla.com"],
+    "expires_in_version": "55",
+    "kind": "count",
+    "description": "Attempt to notify ServiceWorker of push notification."
+  },
+  "PUSH_API_NOTIFY_REGISTRATION_LOST": {
+    "alert_emails": ["push@mozilla.com"],
+    "expires_in_version": "55",
+    "kind": "count",
+    "description": "Attempt to notify ServiceWorker of push notification resubscription."
+  },
   "FXA_CONFIGURED": {
     "alert_emails": ["fx-team@mozilla.com"],
     "expires_in_version": "45",
     "kind": "flag",
     "releaseChannelCollection": "opt-out",
     "description": "If the user is signed in to a Firefox Account on this device"
   },
   "FXA_UNVERIFIED_ACCOUNT_ERRORS": {