Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Commit

Permalink
fix(telemetry): #3118 workaround 500s apparent topsites_first_paint_ts
Browse files Browse the repository at this point in the history
  • Loading branch information
dmose authored and Mardak committed Aug 11, 2017
1 parent 58442c2 commit 8022c34
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 12 deletions.
10 changes: 9 additions & 1 deletion system-addon/lib/TelemetryFeed.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
}

Expand Down
33 changes: 22 additions & 11 deletions system-addon/test/unit/lib/TelemetryFeed.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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"));
Expand Down

0 comments on commit 8022c34

Please sign in to comment.