From 8022c3419d327fc47e584b2f811c5baaccd0ab34 Mon Sep 17 00:00:00 2001
From: Dan Mosedale <dmose2@gmail.com>
Date: Wed, 9 Aug 2017 20:57:08 -0700
Subject: [PATCH] fix(telemetry): #3118 workaround 500s apparent
 topsites_first_paint_ts

---
 system-addon/lib/TelemetryFeed.jsm            | 10 +++++-
 .../test/unit/lib/TelemetryFeed.test.js       | 33 ++++++++++++-------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/system-addon/lib/TelemetryFeed.jsm b/system-addon/lib/TelemetryFeed.jsm
index f8c0906fe2..920543846d 100644
--- a/system-addon/lib/TelemetryFeed.jsm
+++ b/system-addon/lib/TelemetryFeed.jsm
@@ -200,7 +200,6 @@ this.TelemetryFeed = class TelemetryFeed {
         break;
       case at.NEW_TAB_INIT:
         this.addSession(au.getPortIdOfSender(action));
-        this.setLoadTriggerInfo(au.getPortIdOfSender(action));
         break;
       case at.NEW_TAB_UNLOAD:
         this.endSession(au.getPortIdOfSender(action));
@@ -238,6 +237,15 @@ this.TelemetryFeed = class TelemetryFeed {
     // get blows up.
     let session = this.sessions.get(port);
 
+    // Partial workaround for #3118; avoids the worst incorrect associations of
+    // times with browsers, by associating the load trigger with the visibility
+    // event as the user is most likely associating the trigger to the tab just
+    // shown. This helps avoid associateing with a preloaded browser as those
+    // don't get the event until shown. Better fix for more cases forthcoming.
+    if (data.visibility_event_rcvd_ts) {
+      this.setLoadTriggerInfo(port);
+    }
+
     Object.assign(session.perf, data);
   }
 
diff --git a/system-addon/test/unit/lib/TelemetryFeed.test.js b/system-addon/test/unit/lib/TelemetryFeed.test.js
index a0de0478d7..a6d7203c01 100644
--- a/system-addon/test/unit/lib/TelemetryFeed.test.js
+++ b/system-addon/test/unit/lib/TelemetryFeed.test.js
@@ -303,7 +303,29 @@ describe("TelemetryFeed", () => {
 
       assert.include(instance.sessions.get("port123").perf, data);
     });
+
+    it("should call setLoadTriggerInfo if data has visibility_event_rcvd_ts", () => {
+      sandbox.stub(instance, "setLoadTriggerInfo");
+      instance.addSession("port123");
+      const data = {visibility_event_rcvd_ts: 444455};
+
+      instance.saveSessionPerfData("port123", data);
+
+      assert.calledOnce(instance.setLoadTriggerInfo);
+      assert.calledWithExactly(instance.setLoadTriggerInfo, "port123");
+      assert.include(instance.sessions.get("port123").perf, data);
+    });
+
+    it("shouldn't call setLoadTriggerInfo if data has no visibility_event_rcvd_ts", () => {
+      sandbox.stub(instance, "setLoadTriggerInfo");
+      instance.addSession("port123");
+
+      instance.saveSessionPerfData("port123", {monkeys_ts: 444455});
+
+      assert.notCalled(instance.setLoadTriggerInfo);
+    });
   });
+
   describe("#uninit", () => {
     it("should call .telemetrySender.uninit", () => {
       const stub = sandbox.stub(instance.telemetrySender, "uninit");
@@ -340,17 +362,6 @@ describe("TelemetryFeed", () => {
       assert.calledOnce(stub);
       assert.calledWith(stub, "port123");
     });
-    it("should call .setLoadTriggerInfo() on NEW_TAB_INIT action", () => {
-      const stub = sandbox.stub(instance, "setLoadTriggerInfo");
-
-      instance.onAction(ac.SendToMain({
-        type: at.NEW_TAB_INIT,
-        data: {}
-      }, "port123"));
-
-      assert.calledOnce(stub);
-      assert.calledWith(stub, "port123");
-    });
     it("should call .endSession() on a NEW_TAB_UNLOAD action", () => {
       const stub = sandbox.stub(instance, "endSession");
       instance.onAction(ac.SendToMain({type: at.NEW_TAB_UNLOAD}, "port123"));