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"));