Bug 1528080 - Telemetry for shield histogram reports per page load. r=johannh

The shield historgram was reporting once per content blocking event, now it reports per page load.

Differential Revision: https://phabricator.services.mozilla.com/D24017

--HG--
extra : moz-landing-system : lando

browser/base/content/browser-contentblocking.js

@@ -1056,7 +1056,10 @@ var ContentBlocking = {
     Services.telemetry.getHistogramById("FINGERPRINTERS_BLOCKED_COUNT").add(value);
   },
 
+  // This triggers from top level location changes.
   onLocationChange() {
+    // Reset blocking and exception status so that we can send telemetry
+    this.hadShieldState = false;
     let baseURI = this._baseURIForChannelClassifier;
 
     // Don't deal with about:, file: etc.
@@ -1069,6 +1072,7 @@ var ContentBlocking = {
     // Add to telemetry per page load as a baseline measurement.
     this.fingerprintersHistogramAdd("pageLoad");
     this.cryptominersHistogramAdd("pageLoad");
+    this.shieldHistogramAdd(0);
   },
 
   onContentBlockingEvent(event, webProgress, isSimulated) {
@@ -1147,13 +1151,18 @@ var ContentBlocking = {
 
     if (hasException) {
       this.iconBox.setAttribute("tooltiptext", this.strings.disabledTooltipText);
-      this.shieldHistogramAdd(1);
+      if (!this.hadShieldState && !isSimulated) {
+        this.hadShieldState = true;
+        this.shieldHistogramAdd(1);
+      }
     } else if (anyBlocking) {
       this.iconBox.setAttribute("tooltiptext", this.strings.activeTooltipText);
-      this.shieldHistogramAdd(2);
+      if (!this.hadShieldState && !isSimulated) {
+        this.hadShieldState = true;
+        this.shieldHistogramAdd(2);
+      }
     } else {
       this.iconBox.removeAttribute("tooltiptext");
-      this.shieldHistogramAdd(0);
     }
 
     // We report up to one instance of fingerprinting and cryptomining

browser/base/content/test/trackingUI/browser_trackingUI_telemetry.js

@@ -56,24 +56,22 @@ add_task(async function testShieldHistogram() {
   is(getShieldCounts()[0], 1, "Page loads without tracking");
 
   await promiseTabLoadEvent(tab, TRACKING_PAGE);
-  // Note that right now the shield histogram is not measuring what
-  // you might think.  Since onContentBlockingEvent fires twice for a tracking page,
-  // the total page loads count is double counting, and the shield count
-  // (which is meant to measure times when the shield wasn't shown) fires even
-  // when tracking elements exist on the page.
-  todo_is(getShieldCounts()[0], 1, "FIXME: TOTAL PAGE LOADS WITHOUT TRACKING IS DOUBLE COUNTING");
+  is(getShieldCounts()[0], 2, "Adds one more page load");
+  is(getShieldCounts()[2], 1, "Counts one instance of the shield being shown");
 
   info("Disable TP for the page (which reloads the page)");
   let tabReloadPromise = promiseTabLoadEvent(tab);
   document.querySelector("#tracking-action-unblock").doCommand();
   await tabReloadPromise;
-  todo_is(getShieldCounts()[0], 1, "FIXME: TOTAL PAGE LOADS WITHOUT TRACKING IS DOUBLE COUNTING");
+  is(getShieldCounts()[0], 3, "Adds one more page load");
+  is(getShieldCounts()[1], 1, "Counts one instance of the shield being crossed out");
 
   info("Re-enable TP for the page (which reloads the page)");
   tabReloadPromise = promiseTabLoadEvent(tab);
   document.querySelector("#tracking-action-block").doCommand();
   await tabReloadPromise;
-  todo_is(getShieldCounts()[0], 1, "FIXME: TOTAL PAGE LOADS WITHOUT TRACKING IS DOUBLE COUNTING");
+  is(getShieldCounts()[0], 4, "Adds one more page load");
+  is(getShieldCounts()[2], 2, "Adds one more instance of the shield being shown");
 
   gBrowser.removeCurrentTab();
 

toolkit/components/telemetry/Histograms.json

@@ -11412,7 +11412,7 @@
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 4,
-    "description": "Status of the shield icon for each top-level pageload (outside of Private Browsing mode) when tracking protection is enabled (0 = shield not shown because no trackers were detected, 1 = shield crossed out because TP was disabled on this page by the user, 2 = shield shown because a tracker was blocked)",
+    "description": "Status of the shield icon for each top-level pageload (outside of Private Browsing mode) when tracking protection is enabled (0 = there was a page load, this is used as a baseline, 1 = shield crossed out because CB was disabled on this page by the user, 2 = shield shown because some content was blocked)",
     "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
     "bug_numbers": [1058133]
   },