fix(addon): #759 Add session_id for all pings.

lib/TabTracker.js

@@ -84,6 +84,7 @@ TabTracker.prototype = {
     payload.addon_version = self.version;
     payload.locale = Locale.getLocale();
     payload.page = this._resolvePageType(url);
+    payload.session_id = this._tabData.session_id;
   },
 
   uninit() {
@@ -129,6 +130,15 @@ TabTracker.prototype = {
     return Object.assign({}, eventData, {event_id: String(uuid())});
   },
 
+  _initTabSession(tab, loadReason) {
+    // For session IDs that were set on tab open, let's make sure we
+    // don't overwrite them.
+    this._tabData.session_id = this._tabData.session_id || String(uuid());
+    this._tabData.url = tab.url;
+    this._tabData.tab_id = tab.id;
+    this._tabData.load_reason = loadReason;
+  },
+
   navigateAwayFromPage(tab, reason) {
     // we can't use tab.url, because it's pointing to a new url of the page
     // we have to use the URL stored in this._openTabs object
@@ -141,9 +151,8 @@ TabTracker.prototype = {
     if (!this._tabData.tab_id) {
       // We're navigating away from an activity streams page that
       // didn't even load yet. Let's say it's been active for 0 seconds.
-      this._tabData.url = tab.url;
-      this._tabData.tab_id = tab.id;
-      this._tabData.load_reason = this._tabData.load_reason || "none"; // Page didn't load at all.
+      // Note: none is for if the page didn't load at all.
+      this._initTabSession(tab, this._tabData.load_reason || "none");
       this._tabData.session_duration = 0;
       delete this._tabData.start_time;
       Services.obs.notifyObservers(null, COMPLETE_NOTIF, JSON.stringify(this._tabData));
@@ -164,8 +173,6 @@ TabTracker.prototype = {
     // already ended, likely with an 'unfocus' unload reason.
     if (this.isActivityStreamsURL(tab.url) && tabs.activeTab.id === tab.id) {
       if (!this._tabData.url) {
-        this._tabData.url = tab.url;
-        this._tabData.tab_id = tab.id;
         this._tabData.load_reason = "newtab";
       } else if (!this._tabData.session_duration) {
         // The page content has been reloaded but a total time wasn't set.
@@ -192,10 +199,9 @@ TabTracker.prototype = {
       // but also from "activate" event, when the tab gains focus, in which case
       // we need to restore tab_id and url, because they could have been errased
       // by a call navigateAwayFromPage() caused by another tab
-      this._tabData.load_reason = this._tabData.load_reason || "focus";
+      this._initTabSession(tab, this._tabData.load_reason || "focus");
       this._tabData.start_time = Date.now();
-      this._tabData.tab_id = tab.id;
-      this._tabData.url = tab.url;
+
       // URL stored in this._openTabs object keeps the previous URL after the tab.url
       // is replaced with a different page URL, as in click action of page reload
       this._openTabs[tab.id].url = tab.url;
@@ -246,6 +252,10 @@ TabTracker.prototype = {
     // update history and bookmark sizes
     this._placesQueries.getHistorySize().then(size => {this._tabData.total_history_size = size;});
     this._placesQueries.getBookmarksSize().then(size => {this._tabData.total_bookmarks = size;});
+
+    // Some performance pings are sent before a tab is loaded. Let's make sure we have
+    // session id available in advance for those pings.
+    this._tabData.session_id = String(uuid());
   },
 
   _onPrefChange() {

test/test-TabTracker.js

@@ -13,7 +13,7 @@ Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/ClientID.jsm");
 
 const EXPECTED_KEYS = ["url", "tab_id", "session_duration", "client_id", "unload_reason", "addon_version",
-                       "page", "load_reason", "locale", "total_history_size", "total_bookmarks", "action"];
+                       "page", "load_reason", "locale", "total_history_size", "total_bookmarks", "action", "session_id"];
 
 let ACTIVITY_STREAMS_URL;
 let app;
@@ -348,7 +348,7 @@ exports.test_TabTracker_action_pings = function*(assert) {
       data: {
         source: "topsites",
         action_position: 3,
-        event: "click"
+        event: "CLICK"
       }
     }
   };
@@ -359,13 +359,29 @@ exports.test_TabTracker_action_pings = function*(assert) {
   for (let key of additionalKeys) {
     assert.ok(pingData[key], `The ping has the additional key ${key}`);
   }
-  assert.deepEqual(eventData.msg.data, pingData, "We receive the expected ping data.");
+  assert.deepEqual(eventData.msg.data.source, pingData.source, "the ping has the correct source");
+  assert.deepEqual(eventData.msg.data.event, pingData.event, "the ping has the correct event");
+  assert.deepEqual(eventData.msg.data.action_position, pingData.action_position, "the ping has the correct action_position");
 };
 
 exports.test_TabTracker_unload_reason_with_user_action = function*(assert) {
   let events = ["CLICK", "SEARCH"];
   for (let event of events) {
+    let openTab;
+    let sessionPingData = [];
+    let pingsSentPromise = createPingSentPromise(sessionPingData, 1);
+    let tabOpenedPromise = new Promise(resolve => {
+      let onOpen = function(tab) {
+        openTab = tab;
+        tabs.removeListener("pageshow", onOpen);
+        resolve();
+      };
+      tabs.on("pageshow", onOpen);
+    });
+
     tabs.open(ACTIVITY_STREAMS_URL);
+    yield tabOpenedPromise;
+
     let userEventPromise = new Promise(resolve => {
       function observe(subject, topic, data) {
         if (topic === "user-action-event") {
@@ -387,15 +403,23 @@ exports.test_TabTracker_unload_reason_with_user_action = function*(assert) {
     };
     app._handleUserEvent("NOTIFY_USER_EVENT", eventData);
 
-    let pingData = yield userEventPromise;
+    let eventPingData = yield userEventPromise;
     let additionalKeys = ["client_id", "addon_version", "locale", "action", "tab_id", "page"];
     for (let key of additionalKeys) {
-      assert.ok(pingData[key], `The ping has the additional key ${key}`);
+      assert.ok(eventPingData[key], `The ping has the additional key ${key}`);
     }
-    assert.deepEqual(eventData.msg.data, pingData, "We receive the expected ping data.");
+    assert.deepEqual(eventData.msg.data, eventPingData, "We receive the expected ping data.");
+
+    let tabClosedPromise = new Promise(resolve => {
+      openTab.close(() => {
+        resolve();
+      });
+    });
+
+    yield tabClosedPromise;
+    yield pingsSentPromise;
 
-    pingData = yield waitForPageShowAndSessionComplete();
-    checkLoadUnloadReasons(assert, [pingData], ["newtab"], [event.toLowerCase()], false);
+    checkLoadUnloadReasons(assert, sessionPingData, ["newtab"], [event.toLowerCase()], false);
   }
 };