Bug 1548520 - Add user telemetry for Bookmark Panel messages r=nanj
authorAndrei Oprea <andrei.br92@gmail.com>
Wed, 08 May 2019 15:46:42 +0000
changeset 531898 a371ed7348b66078d451235a233f4966a0332f8f
parent 531897 e663519fd15dfdc0bfa7b5c38163ef110edc3264
child 531899 9f6d0100dec9ef0b8dc7ff7a4dc9f81364812ebc
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnanj
bugs1548520
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1548520 - Add user telemetry for Bookmark Panel messages r=nanj Differential Revision: https://phabricator.services.mozilla.com/D30176
browser/components/newtab/lib/BookmarkPanelHub.jsm
browser/components/newtab/test/unit/lib/BookmarkPanelHub.test.js
--- a/browser/components/newtab/lib/BookmarkPanelHub.jsm
+++ b/browser/components/newtab/lib/BookmarkPanelHub.jsm
@@ -4,16 +4,18 @@
 "use strict";
 
 ChromeUtils.defineModuleGetter(this, "DOMLocalization",
   "resource://gre/modules/DOMLocalization.jsm");
 ChromeUtils.defineModuleGetter(this, "FxAccounts",
   "resource://gre/modules/FxAccounts.jsm");
 ChromeUtils.defineModuleGetter(this, "Services",
   "resource://gre/modules/Services.jsm");
+ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
+  "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 class _BookmarkPanelHub {
   constructor() {
     this._id = "BookmarkPanelHub";
     this._trigger = {id: "bookmark-panel"};
     this._handleMessageRequest = null;
     this._addImpression = null;
     this._dispatch = null;
@@ -57,17 +59,17 @@ class _BookmarkPanelHub {
    * to ASRouter. Caches only 1 request, unique identifier is `request.url`.
    * Caching ensures we don't duplicate requests and telemetry pings.
    * Return value is important for the caller to know if a message will be
    * shown.
    *
    * @returns {obj|null} response object or null if no messages matched
    */
   async messageRequest(target, win) {
-    if (this._response && this._response.url === target.url && this._response.content) {
+    if (this._response && this._response.win === win && this._response.url === target.url && this._response.content) {
       this.showMessage(this._response.content, target, win);
       return true;
     }
 
     const response = await this._handleMessageRequest(this._trigger);
 
     return this.onResponse(response, target, win);
   }
@@ -83,17 +85,17 @@ class _BookmarkPanelHub {
       target,
       win,
       url: target.url,
     };
 
     if (response && response.content) {
       this.showMessage(response.content, target, win);
       this.sendImpression();
-      this.sendUserEventTelemetry("IMPRESSION");
+      this.sendUserEventTelemetry("IMPRESSION", win);
     } else {
       this.hideMessage(target);
     }
 
     target.infoButton.disabled = !response;
 
     return !!response;
   }
@@ -112,26 +114,26 @@ class _BookmarkPanelHub {
       recommendation.addEventListener("click", async e => {
         target.hidePopup();
         const url = await FxAccounts.config.promiseEmailFirstURI("bookmark");
         win.ownerGlobal.openLinkIn(url, "tabshifted", {
           private: false,
           triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({}),
           csp: null,
         });
-        this.sendUserEventTelemetry("CLICK");
+        this.sendUserEventTelemetry("CLICK", win);
       });
       recommendation.style.color = message.color;
       recommendation.style.background = `-moz-linear-gradient(-45deg, ${message.background_color_1} 0%, ${message.background_color_2} 70%)`;
       const close = createElement("a");
       close.setAttribute("id", "cfrClose");
       close.setAttribute("aria-label", "close");
       this._l10n.setAttributes(close, message.close_button.tooltiptext);
       close.addEventListener("click", e => {
-        this.sendUserEventTelemetry("DISMISS");
+        this.sendUserEventTelemetry("DISMISS", win);
         this.collapseMessage();
         target.close(e);
       });
       const title = createElement("h1");
       title.setAttribute("id", "editBookmarkPanelRecommendationTitle");
       this._l10n.setAttributes(title, message.title);
       const content = createElement("p");
       content.setAttribute("id", "editBookmarkPanelRecommendationContent");
@@ -185,18 +187,21 @@ class _BookmarkPanelHub {
     this.toggleRecommendation(true);
     this.showMessage(message.content, this._response.target, this._response.win);
   }
 
   sendImpression() {
     this._addImpression(this._response);
   }
 
-  sendUserEventTelemetry(event) {
-    this._sendTelemetry({message_id: this._response.id, bucket_id: this._response.id, event});
+  sendUserEventTelemetry(event, win) {
+    // Only send pings for non private browsing windows
+    if (!PrivateBrowsingUtils.isBrowserPrivate(win.ownerGlobal.gBrowser.selectedBrowser)) {
+      this._sendTelemetry({message_id: this._response.id, bucket_id: this._response.id, event});
+    }
   }
 
   _sendTelemetry(ping) {
     this._dispatch({
       type: "DOORHANGER_TELEMETRY",
       data: {action: "cfr_user_event", source: "CFR", ...ping},
     });
   }
--- a/browser/components/newtab/test/unit/lib/BookmarkPanelHub.test.js
+++ b/browser/components/newtab/test/unit/lib/BookmarkPanelHub.test.js
@@ -7,23 +7,27 @@ describe("BookmarkPanelHub", () => {
   let instance;
   let fakeAddImpression;
   let fakeHandleMessageRequest;
   let fakeL10n;
   let fakeMessage;
   let fakeTarget;
   let fakeContainer;
   let fakeDispatch;
+  let fakeWindow;
+  let isBrowserPrivateStub;
   beforeEach(() => {
     sandbox = sinon.createSandbox();
     globals = new GlobalOverrider();
 
     fakeL10n = {setAttributes: sandbox.stub(), translateElements: sandbox.stub()};
     globals.set("DOMLocalization", function() { return fakeL10n; }); // eslint-disable-line prefer-arrow-callback
     globals.set("FxAccounts", {config: {promiseEmailFirstURI: sandbox.stub()}});
+    isBrowserPrivateStub = sandbox.stub().returns(false);
+    globals.set("PrivateBrowsingUtils", {isBrowserPrivate: isBrowserPrivateStub});
 
     instance = new _BookmarkPanelHub();
     fakeAddImpression = sandbox.stub();
     fakeHandleMessageRequest = sandbox.stub();
     fakeMessage = {
       text: "text",
       title: "title",
       link: {
@@ -52,16 +56,17 @@ describe("BookmarkPanelHub", () => {
         querySelector: sandbox.stub(),
         appendChild: sandbox.stub(),
       },
       hidePopup: sandbox.stub(),
       infoButton: {},
       close: sandbox.stub(),
     };
     fakeDispatch = sandbox.stub();
+    fakeWindow = {ownerGlobal: {openLinkIn: sandbox.stub(), gBrowser: {selectedBrowser: "browser"}}};
   });
   afterEach(() => {
     instance.uninit();
     sandbox.restore();
     globals.restore();
   });
   it("should create an instance", () => {
     assert.ok(instance);
@@ -121,36 +126,46 @@ describe("BookmarkPanelHub", () => {
     beforeEach(() => {
       instance.init(fakeHandleMessageRequest, fakeAddImpression, fakeDispatch);
       sandbox.stub(instance, "showMessage");
       sandbox.stub(instance, "sendImpression");
       sandbox.stub(instance, "hideMessage");
       fakeTarget = {infoButton: {disabled: true}};
     });
     it("should show a message when called with a response", () => {
-      instance.onResponse({content: "content"}, fakeTarget, {});
+      instance.onResponse({content: "content"}, fakeTarget, fakeWindow);
 
       assert.calledOnce(instance.showMessage);
-      assert.calledWithExactly(instance.showMessage, "content", fakeTarget, {});
+      assert.calledWithExactly(instance.showMessage, "content", fakeTarget, fakeWindow);
       assert.calledOnce(instance.sendImpression);
     });
     it("should dispatch a user impression", () => {
       sandbox.spy(instance, "sendUserEventTelemetry");
 
-      instance.onResponse({content: "content"}, fakeTarget, {});
+      instance.onResponse({content: "content"}, fakeTarget, fakeWindow);
 
       assert.calledOnce(instance.sendUserEventTelemetry);
-      assert.calledWithExactly(instance.sendUserEventTelemetry, "IMPRESSION");
+      assert.calledWithExactly(instance.sendUserEventTelemetry, "IMPRESSION", fakeWindow);
       assert.calledOnce(fakeDispatch);
 
       const [ping] = fakeDispatch.firstCall.args;
 
       assert.equal(ping.type, "DOORHANGER_TELEMETRY");
       assert.equal(ping.data.event, "IMPRESSION");
     });
+    it("should not dispatch a user impression if the window is private", () => {
+      isBrowserPrivateStub.returns(true);
+      sandbox.spy(instance, "sendUserEventTelemetry");
+
+      instance.onResponse({content: "content"}, fakeTarget, fakeWindow);
+
+      assert.calledOnce(instance.sendUserEventTelemetry);
+      assert.calledWithExactly(instance.sendUserEventTelemetry, "IMPRESSION", fakeWindow);
+      assert.notCalled(fakeDispatch);
+    });
     it("should hide existing messages if no response is provided", () => {
       instance.onResponse(null, fakeTarget);
 
       assert.calledOnce(instance.hideMessage);
       assert.calledWithExactly(instance.hideMessage, fakeTarget);
     });
   });
   describe("#showMessage.collapsed=false", () => {
@@ -170,67 +185,76 @@ describe("BookmarkPanelHub", () => {
     it("should reuse the container", () => {
       fakeTarget.container.querySelector.returns(true);
 
       instance.showMessage(fakeMessage, fakeTarget);
 
       assert.notCalled(fakeTarget.container.appendChild);
     });
     it("should open a tab with FxA signup", async () => {
-      const windowStub = {ownerGlobal: {openLinkIn: sandbox.stub()}};
       fakeTarget.container.querySelector.returns(false);
 
-      instance.showMessage(fakeMessage, fakeTarget, windowStub);
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
       // Call the event listener cb
       await fakeContainer.addEventListener.firstCall.args[1]();
 
-      assert.calledOnce(windowStub.ownerGlobal.openLinkIn);
+      assert.calledOnce(fakeWindow.ownerGlobal.openLinkIn);
     });
     it("should send a click event", async () => {
-      const windowStub = {ownerGlobal: {openLinkIn: sandbox.stub()}};
       sandbox.stub(instance, "sendUserEventTelemetry");
       fakeTarget.container.querySelector.returns(false);
 
-      instance.showMessage(fakeMessage, fakeTarget, windowStub);
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
       // Call the event listener cb
       await fakeContainer.addEventListener.firstCall.args[1]();
 
       assert.calledOnce(instance.sendUserEventTelemetry);
-      assert.calledWithExactly(instance.sendUserEventTelemetry, "CLICK");
+      assert.calledWithExactly(instance.sendUserEventTelemetry, "CLICK", fakeWindow);
+    });
+    it("should send a click event", async () => {
+      sandbox.stub(instance, "sendUserEventTelemetry");
+      fakeTarget.container.querySelector.returns(false);
+
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
+      // Call the event listener cb
+      await fakeContainer.addEventListener.firstCall.args[1]();
+
+      assert.calledOnce(instance.sendUserEventTelemetry);
+      assert.calledWithExactly(instance.sendUserEventTelemetry, "CLICK", fakeWindow);
     });
     it("should collapse the message", () => {
       fakeTarget.container.querySelector.returns(false);
       sandbox.spy(instance, "collapseMessage");
       instance._response.collapsed = false;
 
-      instance.showMessage(fakeMessage, fakeTarget);
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
       // Show message calls it once so we need to reset
       instance.toggleRecommendation.reset();
       // Call the event listener cb
       fakeContainer.addEventListener.secondCall.args[1]();
 
       assert.calledOnce(instance.collapseMessage);
       assert.calledOnce(fakeTarget.close);
       assert.isTrue(instance._response.collapsed);
       assert.calledOnce(instance.toggleRecommendation);
     });
     it("should send a dismiss event", () => {
       sandbox.stub(instance, "sendUserEventTelemetry");
       sandbox.spy(instance, "collapseMessage");
       instance._response.collapsed = false;
 
-      instance.showMessage(fakeMessage, fakeTarget);
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
       // Call the event listener cb
       fakeContainer.addEventListener.secondCall.args[1]();
 
       assert.calledOnce(instance.sendUserEventTelemetry);
-      assert.calledWithExactly(instance.sendUserEventTelemetry, "DISMISS");
+      assert.calledWithExactly(instance.sendUserEventTelemetry, "DISMISS", fakeWindow);
     });
     it("should call toggleRecommendation `true`", () => {
-      instance.showMessage(fakeMessage, fakeTarget);
+      instance.showMessage(fakeMessage, fakeTarget, fakeWindow);
 
       assert.calledOnce(instance.toggleRecommendation);
       assert.calledWithExactly(instance.toggleRecommendation, true);
     });
   });
   describe("#showMessage.collapsed=true", () => {
     beforeEach(() => {
       sandbox.stub(instance, "_response").value({collapsed: true, target: fakeTarget});