feat(mc): Only send one user interaction ping for each top stories ar…

…ticle

system-addon/lib/ActivityStream.jsm

@@ -81,6 +81,18 @@ const PREFS_CONFIG = new Map([
     title: "Show the Top Sites section on the New Tab page",
     value: true
   }],
+  ["impressionStats.clicked", {
+    title: "GUIDs of clicked Top stories items",
+    value: "[]"
+  }],
+  ["impressionStats.blocked", {
+    title: "GUIDs of blocked Top stories items",
+    value: "[]"
+  }],
+  ["impressionStats.pocketed", {
+    title: "GUIDs of pocketed Top stories items",
+    value: "[]"
+  }],
   ["telemetry", {
     title: "Enable system error and usage data collection",
     value: true,

system-addon/lib/TelemetryFeed.jsm

@@ -30,10 +30,77 @@ const USER_PREFS_ENCODING = {
   "feeds.section.topstories": 1 << 2
 };
 
+const IMPRESSION_STATS_RESET_TIME = 60 * 60 * 1000; // 60 minutes
+const PREF_IMPRESSION_STATS_CLICKED = "impressionStats.clicked";
+const PREF_IMPRESSION_STATS_BLOCKED = "impressionStats.blocked";
+const PREF_IMPRESSION_STATS_POCKETED = "impressionStats.pocketed";
+
+/**
+ * A pref persistent GUID set
+ */
+class PersistentGuidSet extends Set {
+  constructor(prefs, prefName) {
+    let guids = [];
+    try {
+      guids = JSON.parse(prefs.get(prefName));
+      if (typeof guids[Symbol.iterator] !== "function") {
+        guids = [];
+        prefs.set(prefName, "[]");
+      }
+    } catch (e) {
+      Cu.reportError(e);
+      prefs.set(prefName, "[]");
+    }
+
+    super(guids);
+
+    this._prefs = prefs;
+    this._prefName = prefName;
+  }
+
+  /**
+   * Add a GUID and persist
+   *
+   * @param {Integer|String} guid a GUID to save
+   * @returns {Boolean} true if the item has been added
+   */
+  save(guid) {
+    if (!this.has(guid)) {
+      this.add(guid);
+      this._prefs.set(this._prefName, JSON.stringify(this.items()));
+      return true;
+    }
+    return false;
+  }
+
+  /**
+   * Clear GUID set and persist
+   */
+  clear() {
+    if (this.size !== 0) {
+      this._prefs.set(this._prefName, "[]");
+      super.clear();
+    }
+  }
+
+  /**
+   * Return GUID set as an array ordered by insertion time
+   */
+  items() {
+    return [...this];
+  }
+}
+
 this.TelemetryFeed = class TelemetryFeed {
   constructor(options) {
     this.sessions = new Map();
     this._prefs = new Prefs();
+    this._impressionStatsLastReset = Date.now();
+    this._impressionStats = {
+      clicked: new PersistentGuidSet(this._prefs, PREF_IMPRESSION_STATS_CLICKED),
+      blocked: new PersistentGuidSet(this._prefs, PREF_IMPRESSION_STATS_BLOCKED),
+      pocketed: new PersistentGuidSet(this._prefs, PREF_IMPRESSION_STATS_POCKETED)
+    };
   }
 
   init() {
@@ -242,6 +309,36 @@ this.TelemetryFeed = class TelemetryFeed {
     this.telemetrySender.sendPing(await eventPromise);
   }
 
+  handleImpressionStats(action) {
+    const payload = action.data;
+    let guidSet;
+    let index;
+
+    if ("click" in payload) {
+      guidSet = this._impressionStats.clicked;
+      index = payload.click;
+    } else if ("block" in payload) {
+      guidSet = this._impressionStats.blocked;
+      index = payload.block;
+    } else if ("pocket" in payload) {
+      guidSet = this._impressionStats.pocketed;
+      index = payload.pocket;
+    }
+
+    // If it is an impression ping, just send it out. For the click, block, and
+    // save to pocket pings, it only sends the first ping for the same article.
+    if (!guidSet || guidSet.save(payload.tiles[index].id)) {
+      this.sendEvent(this.createImpressionStats(action));
+    }
+  }
+
+  resetImpressionStats() {
+    for (const key of Object.keys(this._impressionStats)) {
+      this._impressionStats[key].clear();
+    }
+    this._impressionStatsLastReset = Date.now();
+  }
+
   onAction(action) {
     switch (action.type) {
       case at.INIT:
@@ -256,8 +353,13 @@ this.TelemetryFeed = class TelemetryFeed {
       case at.SAVE_SESSION_PERF_DATA:
         this.saveSessionPerfData(au.getPortIdOfSender(action), action.data);
         break;
+      case at.SYSTEM_TICK:
+        if (Date.now() - this._impressionStatsLastReset >= IMPRESSION_STATS_RESET_TIME) {
+          this.resetImpressionStats();
+        }
+        break;
       case at.TELEMETRY_IMPRESSION_STATS:
-        this.sendEvent(this.createImpressionStats(action));
+        this.handleImpressionStats(action);
         break;
       case at.TELEMETRY_UNDESIRED_EVENT:
         this.sendEvent(this.createUndesiredEvent(action));
@@ -313,4 +415,12 @@ this.TelemetryFeed = class TelemetryFeed {
   }
 };
 
-this.EXPORTED_SYMBOLS = ["TelemetryFeed", "USER_PREFS_ENCODING"];
+this.EXPORTED_SYMBOLS = [
+  "TelemetryFeed",
+  "PersistentGuidSet",
+  "USER_PREFS_ENCODING",
+  "IMPRESSION_STATS_RESET_TIME",
+  "PREF_IMPRESSION_STATS_CLICKED",
+  "PREF_IMPRESSION_STATS_BLOCKED",
+  "PREF_IMPRESSION_STATS_POCKETED"
+];

system-addon/test/unit/lib/TelemetryFeed.test.js

@@ -24,27 +24,37 @@ describe("TelemetryFeed", () => {
     getState() { return {App: {version: "1.0.0", locale: "en-US"}}; }
   };
   let instance;
+  let clock;
   class TelemetrySender {sendPing() {} uninit() {}}
   class PerfService {
     getMostRecentAbsMarkStartByName() { return 1234; }
     mark() {}
     absNow() { return 123; }
   }
   const perfService = new PerfService();
-  const {TelemetryFeed, USER_PREFS_ENCODING} = injector({
+  const {
+    TelemetryFeed,
+    USER_PREFS_ENCODING,
+    IMPRESSION_STATS_RESET_TIME,
+    PREF_IMPRESSION_STATS_CLICKED,
+    PREF_IMPRESSION_STATS_BLOCKED,
+    PREF_IMPRESSION_STATS_POCKETED
+  } = injector({
     "lib/TelemetrySender.jsm": {TelemetrySender},
     "common/PerfService.jsm": {perfService}
   });
 
   beforeEach(() => {
     globals = new GlobalOverrider();
     sandbox = globals.sandbox;
+    clock = sinon.useFakeTimers();
     globals.set("ClientID", {getClientID: sandbox.spy(async () => FAKE_TELEMETRY_ID)});
     globals.set("gUUIDGenerator", {generateUUID: () => FAKE_UUID});
     instance = new TelemetryFeed();
     instance.store = store;
   });
   afterEach(() => {
+    clock.restore();
     globals.restore();
     FakePrefs.prototype.prefs = {};
   });
@@ -410,7 +420,31 @@ describe("TelemetryFeed", () => {
         instance.browserOpenNewtabStart, "browser-open-newtab-start");
     });
   });
+  describe("#resetImpressionStats", () => {
+    beforeEach(() => {
+      FakePrefs.prototype.prefs = {};
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_CLICKED] = "[10000]";
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_BLOCKED] = "[10001]";
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_POCKETED] = "[10002]";
+    });
+    it("should reset all the GUID sets for impression stats", () => {
+      const lastResetTime = instance._impressionStatsLastReset;
+      // Haven't restored the clock yet, we have to manually tick the clock.
+      clock.tick(IMPRESSION_STATS_RESET_TIME);
+      instance.resetImpressionStats();
+      for (const key of Object.keys(instance._impressionStats)) {
+        assert.equal(instance._impressionStats[key].size, 0);
+      }
+      assert.isAbove(instance._impressionStatsLastReset, lastResetTime);
+    });
+  });
   describe("#onAction", () => {
+    beforeEach(() => {
+      FakePrefs.prototype.prefs = {};
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_CLICKED] = "[]";
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_BLOCKED] = "[]";
+      FakePrefs.prototype.prefs[PREF_IMPRESSION_STATS_POCKETED] = "[]";
+    });
     it("should call .init() on an INIT action", () => {
       const stub = sandbox.stub(instance, "init");
       instance.onAction({type: at.INIT});
@@ -469,10 +503,121 @@ describe("TelemetryFeed", () => {
     it("should send an event on a TELEMETRY_IMPRESSION_STATS action", () => {
       const sendEvent = sandbox.stub(instance, "sendEvent");
       const eventCreator = sandbox.stub(instance, "createImpressionStats");
-      const action = {type: at.TELEMETRY_IMPRESSION_STATS};
+      const action = {type: at.TELEMETRY_IMPRESSION_STATS, data: {}};
       instance.onAction(action);
       assert.calledWith(eventCreator, action);
       assert.calledWith(sendEvent, eventCreator.returnValue);
     });
+    it("should call .resetImpressionStats on a SYSTEM_TICK action", () => {
+      const resetImpressionStats = sandbox.stub(instance, "resetImpressionStats");
+
+      instance.onAction({type: at.SYSTEM_TICK});
+      assert.notCalled(resetImpressionStats);
+
+      clock.tick(IMPRESSION_STATS_RESET_TIME);
+      instance.onAction({type: at.SYSTEM_TICK});
+      assert.calledOnce(resetImpressionStats);
+    });
+    it("should not send two click pings for the same article", async () => {
+      const sendEvent = sandbox.stub(instance, "sendEvent");
+      const tiles = [{id: 10001, pos: 2}];
+      const data = {tiles, click: 0};
+      const action = {type: at.TELEMETRY_IMPRESSION_STATS, data};
+
+      instance.onAction(action);
+      instance.onAction(action);
+      assert.calledOnce(sendEvent);
+    });
+    it("should not send two block pings for the same article", async () => {
+      const sendEvent = sandbox.stub(instance, "sendEvent");
+      const tiles = [{id: 10001, pos: 2}];
+      const data = {tiles, block: 0};
+      const action = {type: at.TELEMETRY_IMPRESSION_STATS, data};
+
+      instance.onAction(action);
+      instance.onAction(action);
+      assert.calledOnce(sendEvent);
+    });
+    it("should not send two save to pocket pings for the same article", async () => {
+      const sendEvent = sandbox.stub(instance, "sendEvent");
+      const tiles = [{id: 10001, pos: 2}];
+      const data = {tiles, pocket: 0};
+      const action = {type: at.TELEMETRY_IMPRESSION_STATS, data};
+
+      instance.onAction(action);
+      instance.onAction(action);
+      assert.calledOnce(sendEvent);
+    });
+  });
+});
+
+describe("PersistentGuidSet", () => {
+  const {PersistentGuidSet} = injector({});
+
+  afterEach(() => {
+    FakePrefs.prototype.prefs = {};
+  });
+  describe("#init", () => {
+    it("should initialized empty", () => {
+      let guidSet;
+
+      guidSet = new PersistentGuidSet(new FakePrefs(), "test.guidSet");
+      assert.equal(guidSet.size, 0);
+      assert.deepEqual(guidSet.items(), []);
+    });
+    it("should initialized from pref", () => {
+      let guidSet;
+
+      FakePrefs.prototype.prefs = {"test.guidSet": JSON.stringify([10000])};
+      guidSet = new PersistentGuidSet(new FakePrefs(), "test.guidSet");
+      assert.equal(guidSet.size, 1);
+      assert.isTrue(guidSet.has(10000));
+      assert.deepEqual(guidSet.items(), [10000]);
+    });
+    it("should initialized empty with invalid pref", () => {
+      let guidSet;
+
+      FakePrefs.prototype.prefs = {"test.guidSet": 10000};
+      guidSet = new PersistentGuidSet(new FakePrefs(), "test.guidSet");
+      assert.equal(guidSet.size, 0);
+      assert.deepEqual(guidSet.items(), []);
+    });
+  });
+  describe("#save", () => {
+    it("should save the new GUID", () => {
+      let guidSet;
+      let prefs = new FakePrefs();
+
+      guidSet = new PersistentGuidSet(prefs, "test.guidSet");
+      guidSet.save("10000");
+      guidSet.save("10001");
+      assert.equal(guidSet.size, 2);
+      assert.deepEqual(guidSet.items(), ["10000", "10001"]);
+      assert.equal(prefs.get("test.guidSet"), "[\"10000\",\"10001\"]");
+    });
+    it("should not save the same GUID twice", () => {
+      let guidSet;
+      let prefs = new FakePrefs();
+
+      guidSet = new PersistentGuidSet(prefs, "test.guidSet");
+      guidSet.save("10000");
+      assert.isFalse(guidSet.save("10000"));
+      assert.equal(guidSet.size, 1);
+      assert.deepEqual(guidSet.items(), ["10000"]);
+      assert.equal(prefs.get("test.guidSet"), "[\"10000\"]");
+    });
+  });
+  describe("#clear", () => {
+    it("should clear the GUID set", () => {
+      let guidSet;
+
+      guidSet = new PersistentGuidSet(new FakePrefs(), "test.guidSet");
+      guidSet.save("10000");
+      guidSet.save("10001");
+      assert.equal(guidSet.size, 2);
+      guidSet.clear();
+      assert.equal(guidSet.size, 0);
+      assert.deepEqual(guidSet.items(), []);
+    });
   });
 });