Bug 1226629 - Increment "urlbar" BrowserUITelemetry/FHR for all searchengine results clicked in the urlbar. r=mak

browser/base/content/test/general/browser.ini

@@ -472,6 +472,7 @@ skip-if = os == "linux" || e10s # Bug 1073339 - Investigate autocomplete test un
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarSearchSuggestions.js]
 [browser_urlbarSearchSuggestionsNotification.js]
+[browser_urlbarSearchTelemetry.js]
 [browser_urlbarStop.js]
 [browser_urlbarTrimURLs.js]
 [browser_urlbar_autoFill_backspaced.js]

browser/base/content/test/general/browser_aboutHome.js

@@ -114,7 +114,7 @@ var gTests = [
 
     // Get the current number of recorded searches.
     let searchStr = "a search";
-    getNumberOfSearches(engineName).then(num => {
+    getNumberOfSearchesInFHR(engineName, "abouthome").then(num => {
       numSearchesBefore = num;
 
       info("Perform a search.");
@@ -126,7 +126,7 @@ var gTests = [
                       getSubmission(searchStr, null, "homepage").
                       uri.spec;
     let loadPromise = waitForDocLoadAndStopIt(expectedURL).then(() => {
-      getNumberOfSearches(engineName).then(num => {
+      getNumberOfSearchesInFHR(engineName, "abouthome").then(num => {
         is(num, numSearchesBefore + 1, "One more search recorded.");
         searchEventDeferred.resolve();
       });
@@ -601,58 +601,6 @@ function promiseSetupSnippetsMap(aTab, aSetupFn)
   return deferred.promise;
 }
 
-/**
- * Retrieves the number of about:home searches recorded for the current day.
- *
- * @param aEngineName
- *        name of the setup search engine.
- *
- * @return {Promise} Returns a promise resolving to the number of searches.
- */
-function getNumberOfSearches(aEngineName) {
-  let reporter = Components.classes["@mozilla.org/datareporting/service;1"]
-                                   .getService()
-                                   .wrappedJSObject
-                                   .healthReporter;
-  ok(reporter, "Health Reporter instance available.");
-
-  return reporter.onInit().then(function onInit() {
-    let provider = reporter.getProvider("org.mozilla.searches");
-    ok(provider, "Searches provider is available.");
-
-    let m = provider.getMeasurement("counts", 3);
-    return m.getValues().then(data => {
-      let now = new Date();
-      let yday = new Date(now);
-      yday.setDate(yday.getDate() - 1);
-
-      // Add the number of searches recorded yesterday to the number of searches
-      // recorded today. This makes the test not fail intermittently when it is
-      // run at midnight and we accidentally compare the number of searches from
-      // different days. Tests are always run with an empty profile so there
-      // are no searches from yesterday, normally. Should the test happen to run
-      // past midnight we make sure to count them in as well.
-      return getNumberOfSearchesByDate(aEngineName, data, now) +
-             getNumberOfSearchesByDate(aEngineName, data, yday);
-    });
-  });
-}
-
-function getNumberOfSearchesByDate(aEngineName, aData, aDate) {
-  if (aData.days.hasDay(aDate)) {
-    let id = Services.search.getEngineByName(aEngineName).identifier;
-
-    let day = aData.days.getDay(aDate);
-    let field = id + ".abouthome";
-
-    if (day.has(field)) {
-      return day.get(field) || 0;
-    }
-  }
-
-  return 0; // No records found.
-}
-
 function waitForLoad(cb) {
   let browser = gBrowser.selectedBrowser;
   browser.addEventListener("load", function listener() {

browser/base/content/test/general/browser_urlbarSearchTelemetry.js

@@ -0,0 +1,177 @@
+"use strict";
+
+Cu.import("resource:///modules/BrowserUITelemetry.jsm");
+
+const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
+const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
+
+// Must run first.
+add_task(function* prepare() {
+  Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
+  let engine = yield promiseNewSearchEngine(TEST_ENGINE_BASENAME);
+  let oldCurrentEngine = Services.search.currentEngine;
+  Services.search.currentEngine = engine;
+  registerCleanupFunction(function* () {
+    Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF);
+    Services.search.currentEngine = oldCurrentEngine;
+
+    // Clicking urlbar results causes visits to their associated pages, so clear
+    // that history now.
+    yield PlacesTestUtils.clearHistory();
+
+    // Make sure the popup is closed for the next test.
+    gURLBar.blur();
+    Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+  });
+});
+
+add_task(function* heuristicResult() {
+  yield compareCounts(function* () {
+    yield promiseAutocompleteResultPopup("heuristicResult");
+    let action = getActionAtIndex(0);
+    Assert.ok(!!action, "there should be an action at index 0");
+    Assert.equal(action.type, "searchengine", "type should be searchengine");
+    let item = gURLBar.popup.richlistbox.getItemAtIndex(0);
+    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
+    item.click();
+    yield loadPromise;
+  });
+});
+
+add_task(function* searchSuggestion() {
+  yield compareCounts(function* () {
+    yield promiseAutocompleteResultPopup("searchSuggestion");
+    let idx = getFirstSuggestionIndex();
+    Assert.ok(idx >= 0, "there should be a first suggestion");
+    let item = gURLBar.popup.richlistbox.getItemAtIndex(idx);
+    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
+    item.click();
+    yield loadPromise;
+  });
+});
+
+/**
+ * This does three things: gets current telemetry/FHR counts, calls
+ * clickCallback, gets telemetry/FHR counts again to compare them to the old
+ * counts.
+ *
+ * @param clickCallback Use this to open the urlbar popup and choose and click a
+ *        result.
+ */
+function* compareCounts(clickCallback) {
+  // Search events triggered by clicks (not the Return key in the urlbar) are
+  // recorded in three places:
+  // * BrowserUITelemetry
+  // * Telemetry histogram named "SEARCH_COUNTS"
+  // * FHR
+
+  let engine = Services.search.currentEngine;
+  let engineID = "org.mozilla.testsearchsuggestions";
+
+  // First, get the current counts.
+
+  // BrowserUITelemetry
+  let uiTelemCount = 0;
+  let bucket = BrowserUITelemetry.currentBucket;
+  let events = BrowserUITelemetry.getToolbarMeasures().countableEvents;
+  if (events[bucket] &&
+      events[bucket].search &&
+      events[bucket].search.urlbar) {
+    uiTelemCount = events[bucket].search.urlbar;
+  }
+
+  // telemetry histogram SEARCH_COUNTS
+  let histogramCount = 0;
+  let histogramKey = engineID + ".urlbar";
+  let histogram;
+  try {
+    histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
+  } catch (ex) {
+    // No searches performed yet, not a problem.
+  }
+  if (histogram) {
+    let snapshot = histogram.snapshot();
+    if (histogramKey in snapshot) {
+      histogramCount = snapshot[histogramKey].sum;
+    }
+  }
+
+  // FHR -- first make sure the engine has an identifier so that FHR is happy.
+  Object.defineProperty(engine.wrappedJSObject, "identifier",
+                        { value: engineID });
+  let fhrCount = yield getNumberOfSearchesInFHR(engine.name, "urlbar");
+
+  gURLBar.focus();
+  yield clickCallback();
+
+  // Now get the new counts and compare them to the old.
+
+  // BrowserUITelemetry
+  events = BrowserUITelemetry.getToolbarMeasures().countableEvents;
+  Assert.ok(bucket in events, "bucket should be recorded");
+  events = events[bucket];
+  Assert.ok("search" in events, "search should be recorded");
+  events = events.search;
+  Assert.ok("urlbar" in events, "urlbar should be recorded");
+  Assert.equal(events.urlbar, uiTelemCount + 1,
+               "clicked suggestion should be recorded");
+
+  // telemetry histogram SEARCH_COUNTS
+  histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
+  let snapshot = histogram.snapshot();
+  Assert.ok(histogramKey in snapshot, "histogram with key should be recorded");
+  Assert.equal(snapshot[histogramKey].sum, histogramCount + 1,
+               "histogram sum should be incremented");
+
+  // FHR
+  let newFHRCount = yield getNumberOfSearchesInFHR(engine.name, "urlbar");
+  Assert.equal(newFHRCount, fhrCount + 1, "should be recorded in FHR");
+}
+
+/**
+ * Returns the "action" object at the given index in the urlbar results:
+ * { type, params: {}}
+ *
+ * @param index The index in the urlbar results.
+ * @return An action object, or null if index >= number of results.
+ */
+function getActionAtIndex(index) {
+  let controller = gURLBar.popup.input.controller;
+  if (controller.matchCount <= index) {
+    return null;
+  }
+  let url = controller.getValueAt(index);
+  let mozActionMatch = url.match(/^moz-action:([^,]+),(.*)$/);
+  if (!mozActionMatch) {
+    let msg = "result at index " + index + " is not a moz-action: " + url;
+    Assert.ok(false, msg);
+    throw new Error(msg);
+  }
+  let [, type, paramStr] = mozActionMatch;
+  return {
+    type: type,
+    params: JSON.parse(paramStr),
+  };
+}
+
+/**
+ * Returns the index of the first search suggestion in the urlbar results.
+ *
+ * @return An index, or -1 if there are no search suggestions.
+ */
+function getFirstSuggestionIndex() {
+  let controller = gURLBar.popup.input.controller;
+  let matchCount = controller.matchCount;
+  for (let i = 0; i < matchCount; i++) {
+    let url = controller.getValueAt(i);
+    let mozActionMatch = url.match(/^moz-action:([^,]+),(.*)$/);
+    if (mozActionMatch) {
+      let [, type, paramStr] = mozActionMatch;
+      let params = JSON.parse(paramStr);
+      if (type == "searchengine" && "searchSuggestion" in params) {
+        return i;
+      }
+    }
+  }
+  return -1;
+}

browser/base/content/test/general/head.js

@@ -1202,3 +1202,61 @@ function promiseCrashReport(expectedExtra) {
     }
   });
 }
+
+/**
+ * Retrieves the number of searches recorded in FHR for the current day.
+ *
+ * @param aEngineName
+ *        name of the setup search engine.
+ * @param aSource
+ *        The FHR "source" name for the search, like "abouthome" or "urlbar".
+ *
+ * @return {Promise} Returns a promise resolving to the number of searches.
+ */
+function getNumberOfSearchesInFHR(aEngineName, aSource) {
+  let reporter = Components.classes["@mozilla.org/datareporting/service;1"]
+                                   .getService()
+                                   .wrappedJSObject
+                                   .healthReporter;
+  ok(reporter, "Health Reporter instance available.");
+
+  return reporter.onInit().then(function onInit() {
+    let provider = reporter.getProvider("org.mozilla.searches");
+    ok(provider, "Searches provider is available.");
+
+    let m = provider.getMeasurement("counts", 3);
+    return m.getValues().then(data => {
+      let now = new Date();
+      let yday = new Date(now);
+      yday.setDate(yday.getDate() - 1);
+
+      // Add the number of searches recorded yesterday to the number of searches
+      // recorded today. This makes the test not fail intermittently when it is
+      // run at midnight and we accidentally compare the number of searches from
+      // different days. Tests are always run with an empty profile so there
+      // are no searches from yesterday, normally. Should the test happen to run
+      // past midnight we make sure to count them in as well.
+      return getNumberOfSearchesInFHRByDate(aEngineName, aSource, data, now) +
+             getNumberOfSearchesInFHRByDate(aEngineName, aSource, data, yday);
+    });
+  });
+}
+
+/**
+ * Helper for getNumberOfSearchesInFHR.  You probably don't want to call this
+ * directly.
+ */
+function getNumberOfSearchesInFHRByDate(aEngineName, aSource, aData, aDate) {
+  if (aData.days.hasDay(aDate)) {
+    let id = Services.search.getEngineByName(aEngineName).identifier;
+
+    let day = aData.days.getDay(aDate);
+    let field = id + "." + aSource;
+
+    if (day.has(field)) {
+      return day.get(field) || 0;
+    }
+  }
+
+  return 0; // No records found.
+}

browser/base/content/urlbarBindings.xml

@@ -361,13 +361,7 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
             } else if (action.type == "keyword") {
               url = action.params.url;
             } else if (action.type == "searchengine") {
-              let engine = Services.search.getEngineByName(action.params.engineName);
-              let query = action.params.searchSuggestion ||
-                          action.params.searchQuery;
-              let submission = engine.getSubmission(query, null, "keyword");
-
-              url = submission.uri.spec;
-              postData = submission.postData;
+              [url, postData] = this._parseAndRecordSearchEngineAction(action);
             } else if (action.type == "visiturl") {
               url = action.params.url;
             }
@@ -455,6 +449,19 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
         ]]></body>
       </method>
 
+      <method name="_parseAndRecordSearchEngineAction">
+        <parameter name="action"/>
+        <body><![CDATA[
+          let engine =
+            Services.search.getEngineByName(action.params.engineName);
+          BrowserSearch.recordSearchInHealthReport(engine, "urlbar");
+          let query = action.params.searchSuggestion ||
+                      action.params.searchQuery;
+          let submission = engine.getSubmission(query, null, "keyword");
+          return [submission.uri.spec, submission.postData];
+        ]]></body>
+      </method>
+
       <method name="_canonizeURL">
         <parameter name="aTriggeringEvent"/>
         <parameter name="aCallback"/>
@@ -1455,12 +1462,8 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
                   break;
                 }
                 case "searchengine": {
-                  let engine = Services.search.getEngineByName(action.params.engineName);
-                  let query = action.params.searchSuggestion ||
-                              action.params.searchQuery;
-                  let submission = engine.getSubmission(query, null, "keyword");
-                  url = submission.uri.spec;
-                  options.postData = submission.postData;
+                  [url, options.postData] =
+                    this._parseAndRecordSearchEngineAction(action);
                   break;
                 }
                 default: {

browser/components/nsBrowserGlue.js

@@ -422,39 +422,16 @@ BrowserGlue.prototype = {
         this._dispose();
         break;
       case "keyword-search":
-        // This is very similar to code in
-        // browser.js:BrowserSearch.recordSearchInHealthReport(). The code could
-        // be consolidated if there is will. We need the observer in
-        // nsBrowserGlue to prevent double counting.
-        let win = RecentWindow.getMostRecentBrowserWindow();
-        BrowserUITelemetry.countSearchEvent("urlbar", win.gURLBar.value);
-
+        // This notification is broadcast by the docshell when it "fixes up" a
+        // URI that it's been asked to load into a keyword search.
         let engine = null;
         try {
           engine = subject.QueryInterface(Ci.nsISearchEngine);
         } catch (ex) {
           Cu.reportError(ex);
         }
-
-        win.BrowserSearch.recordSearchInTelemetry(engine, "urlbar");
-#ifdef MOZ_SERVICES_HEALTHREPORT
-        let reporter = Cc["@mozilla.org/datareporting/service;1"]
-                         .getService()
-                         .wrappedJSObject
-                         .healthReporter;
-
-        if (!reporter) {
-          return;
-        }
-
-        reporter.onInit().then(function record() {
-          try {
-            reporter.getProvider("org.mozilla.searches").recordSearch(engine, "urlbar");
-          } catch (ex) {
-            Cu.reportError(ex);
-          }
-        });
-#endif
+        let win = RecentWindow.getMostRecentBrowserWindow();
+        win.BrowserSearch.recordSearchInHealthReport(engine, "urlbar");
         break;
       case "browser-search-engine-modified":
         // Ensure we cleanup the hiddenOneOffs pref when removing