Bug 1005504 - Fix telemetry for application reputation. r=gcp, a=sledru
authorMonica Chew <mmc@mozilla.com>
Mon, 05 May 2014 13:32:56 -0700
changeset 192219 2919cce749c0
parent 192218 1c5aa2062579
child 192220 da8b1c15e2b1
push id3528
push userryanvm@gmail.com
push date2014-05-07 20:48 +0000
Treeherderresults
reviewersgcp, sledru
bugs1005504
milestone30.0
Bug 1005504 - Fix telemetry for application reputation. r=gcp, a=sledru
toolkit/components/downloads/ApplicationReputation.cpp
toolkit/components/downloads/test/unit/test_app_rep.js
toolkit/components/telemetry/Histograms.json
--- a/toolkit/components/downloads/ApplicationReputation.cpp
+++ b/toolkit/components/downloads/ApplicationReputation.cpp
@@ -880,16 +880,18 @@ ApplicationReputationService::QueryReput
     nsIApplicationReputationQuery* aQuery,
     nsIApplicationReputationCallback* aCallback) {
   NS_ENSURE_ARG_POINTER(aQuery);
   NS_ENSURE_ARG_POINTER(aCallback);
 
   Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_COUNT, true);
   nsresult rv = QueryReputationInternal(aQuery, aCallback);
   if (NS_FAILED(rv)) {
+    Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_SHOULD_BLOCK,
+      false);
     aCallback->OnComplete(false, rv);
   }
   return NS_OK;
 }
 
 nsresult ApplicationReputationService::QueryReputationInternal(
   nsIApplicationReputationQuery* aQuery,
   nsIApplicationReputationCallback* aCallback) {
--- a/toolkit/components/downloads/test/unit/test_app_rep.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep.js
@@ -7,16 +7,20 @@
 Cu.import('resource://gre/modules/NetUtil.jsm');
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const gAppRep = Cc["@mozilla.org/downloads/application-reputation-service;1"].
                   getService(Ci.nsIApplicationReputationService);
 let gHttpServ = null;
 let gTables = {};
 
+let ALLOW_LIST = 0;
+let BLOCK_LIST = 1;
+let NO_LIST = 2;
+
 function readFileToString(aFilename) {
   let f = do_get_file(aFilename);
   let stream = Cc["@mozilla.org/network/file-input-stream;1"]
                  .createInstance(Ci.nsIFileInputStream);
   stream.init(f, -1, 0, 0);
   let buf = NetUtil.readInputStreamToString(stream, stream.available());
   return buf;
 }
@@ -72,48 +76,98 @@ function run_test() {
   gHttpServ.registerPathHandler("/download", function(request, response) {
     do_throw("This test should never make a remote lookup");
   });
   gHttpServ.start(4444);
 
   run_next_test();
 }
 
+function check_telemetry(aCount,
+                         aShouldBlockCount,
+                         aListCounts) {
+  let count = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_COUNT")
+                .snapshot();
+  do_check_eq(count.counts[1], aCount);
+  let local = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_LOCAL")
+                .snapshot();
+  do_check_eq(local.counts[ALLOW_LIST], aListCounts[ALLOW_LIST]);
+  do_check_eq(local.counts[BLOCK_LIST], aListCounts[BLOCK_LIST]);
+  do_check_eq(local.counts[NO_LIST], aListCounts[NO_LIST]);
+  let shouldBlock = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_SHOULD_BLOCK")
+                .snapshot();
+  // SHOULD_BLOCK = true
+  do_check_eq(shouldBlock.counts[1], aShouldBlockCount);
+  // Sanity check that SHOULD_BLOCK total adds up to the COUNT.
+  do_check_eq(shouldBlock.counts[0] + shouldBlock.counts[1], aCount);
+}
+
+function get_telemetry_counts() {
+  let count = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_COUNT")
+                .snapshot();
+  let local = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_LOCAL")
+                .snapshot();
+  let shouldBlock = Cc["@mozilla.org/base/telemetry;1"]
+                .getService(Ci.nsITelemetry)
+                .getHistogramById("APPLICATION_REPUTATION_SHOULD_BLOCK")
+                .snapshot();
+  return { total: count.counts[1],
+           shouldBlock: shouldBlock.counts[1],
+           listCounts: local.counts };
+}
+
 add_test(function test_nullSourceURI() {
+  let counts = get_telemetry_counts();
   gAppRep.queryReputation({
     // No source URI
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     do_check_eq(Cr.NS_ERROR_UNEXPECTED, aStatus);
     do_check_false(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock, counts.listCounts);
     run_next_test();
   });
 });
 
 add_test(function test_nullCallback() {
+  let counts = get_telemetry_counts();
   try {
     gAppRep.queryReputation({
       sourceURI: createURI("http://example.com"),
       fileSize: 12,
     }, null);
     do_throw("Callback cannot be null");
   } catch (ex if ex.result == Cr.NS_ERROR_INVALID_POINTER) {
+    // We don't even increment the count here, because there's no callback.
+    check_telemetry(counts.total, counts.shouldBlock, counts.listCounts);
     run_next_test();
   }
 });
 
 add_test(function test_disabled() {
+  let counts = get_telemetry_counts();
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL", "");
   gAppRep.queryReputation({
     sourceURI: createURI("http://example.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     // We should be getting NS_ERROR_NOT_AVAILABLE if the service is disabled
     do_check_eq(Cr.NS_ERROR_NOT_AVAILABLE, aStatus);
     do_check_false(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock, counts.listCounts);
     run_next_test();
   });
 });
 
 // Set up the local whitelist.
 add_test(function test_local_list() {
   // Construct a response with redirect urls.
   function processUpdateRequest() {
@@ -163,58 +217,74 @@ add_test(function test_local_list() {
     "goog-downloadwhite-digest256,goog-badbinurl-shavar",
     "goog-downloadwhite-digest256,goog-badbinurl-shavar;\n",
     updateSuccess, handleError, handleError);
 });
 
 add_test(function test_unlisted() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
+  let counts = get_telemetry_counts();
+  let listCounts = counts.listCounts;
+  listCounts[NO_LIST]++;
   gAppRep.queryReputation({
     sourceURI: createURI("http://example.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_false(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock, listCounts);
     run_next_test();
   });
 });
 
 add_test(function test_local_blacklist() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
+  let counts = get_telemetry_counts();
+  let listCounts = counts.listCounts;
+  listCounts[BLOCK_LIST]++;
   gAppRep.queryReputation({
     sourceURI: createURI("http://blocklisted.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_true(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock + 1, listCounts);
     run_next_test();
   });
 });
 
 add_test(function test_referer_blacklist() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
+  let counts = get_telemetry_counts();
+  let listCounts = counts.listCounts;
+  listCounts[BLOCK_LIST]++;
   gAppRep.queryReputation({
     sourceURI: createURI("http://example.com"),
     referrerURI: createURI("http://blocklisted.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_true(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock + 1, listCounts);
     run_next_test();
   });
 });
 
 add_test(function test_blocklist_trumps_allowlist() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
+  let counts = get_telemetry_counts();
+  let listCounts = counts.listCounts;
+  listCounts[BLOCK_LIST]++;
   gAppRep.queryReputation({
     sourceURI: createURI("http://whitelisted.com"),
     referrerURI: createURI("http://blocklisted.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_true(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock + 1, listCounts);
     run_next_test();
   });
 });
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -42,17 +42,17 @@
   "APPLICATION_REPUTATION_SERVER": {
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 3,
     "description": "Application reputation remote status (0 = OK, 1 = FAIL, 2 = INVALID)"
   },
   "APPLICATION_REPUTATION_COUNT": {
     "expires_in_version": "never",
-    "kind": "flag",
+    "kind": "boolean",
     "description": "Application reputation query count (both local and remote)"
   },
   "BACKGROUNDFILESAVER_THREAD_COUNT": {
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 21,
     "description": "Maximum number of concurrent threads reached during a given download session"
   },