Bug 1226629 - Increment "urlbar" BrowserUITelemetry/FHR for all searchengine results clicked in the urlbar. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Wed, 02 Dec 2015 16:26:07 -0800
changeset 309378 a78f8444400c700be312e2e653d7e2d74994900e
parent 309377 fc6e4f66615258125ef090c4c3b50b42ff28b467
child 309379 d9fd7783ba309ede1264bd64280b37b6632cf19b
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1226629
milestone45.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 1226629 - Increment "urlbar" BrowserUITelemetry/FHR for all searchengine results clicked in the urlbar. r=mak
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_aboutHome.js
browser/base/content/test/general/browser_urlbarSearchTelemetry.js
browser/base/content/test/general/head.js
browser/base/content/urlbarBindings.xml
browser/components/nsBrowserGlue.js
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -467,16 +467,17 @@ skip-if = e10s # Bug 1100700 - test reli
 [browser_urlbarDelete.js]
 [browser_urlbarEnter.js]
 [browser_urlbarEnterAfterMouseOver.js]
 skip-if = os == "linux" || e10s # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s
 [browser_urlbarRevert.js]
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarSearchSuggestions.js]
 [browser_urlbarSearchSuggestionsNotification.js]
+[browser_urlbarSearchTelemetry.js]
 [browser_urlbarStop.js]
 [browser_urlbarTrimURLs.js]
 [browser_urlbar_autoFill_backspaced.js]
 [browser_urlbar_search_healthreport.js]
 [browser_urlbar_searchsettings.js]
 [browser_utilityOverlay.js]
 [browser_viewSourceInTabOnViewSource.js]
 [browser_visibleFindSelection.js]
--- a/browser/base/content/test/general/browser_aboutHome.js
+++ b/browser/base/content/test/general/browser_aboutHome.js
@@ -109,29 +109,29 @@ var gTests = [
     let numSearchesBefore = 0;
     let searchEventDeferred = Promise.defer();
     let doc = gBrowser.contentDocument;
     let engineName = gBrowser.contentWindow.wrappedJSObject.gContentSearchController.defaultEngine.name;
     is(engine.name, engineName, "Engine name in DOM should match engine we just added");
 
     // 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.");
       doc.getElementById("searchText").value = searchStr;
       doc.getElementById("searchSubmit").click();
     });
 
     let expectedURL = Services.search.currentEngine.
                       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();
       });
     });
 
     try {
       yield Promise.all([searchEventDeferred.promise, loadPromise]);
     } catch (ex) {
@@ -596,68 +596,16 @@ function promiseSetupSnippetsMap(aTab, a
       aSnippetsMap.delete("snippets");
       aSetupFn(aSnippetsMap);
       deferred.resolve(aSnippetsMap);
     });
   }, true, true);
   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() {
     if (browser.currentURI.spec == "about:blank")
       return;
     info("Page loaded: " + browser.currentURI.spec);
     browser.removeEventListener("load", listener, true);
 
new file mode 100644
--- /dev/null
+++ b/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;
+}
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@@ -1197,8 +1197,66 @@ function promiseCrashReport(expectedExtr
       let value = extra.getPropertyAsAString(key);
       if (key in expectedExtra) {
         is(value, expectedExtra[key],
            `Crash report had the right extra value for ${key}`);
       }
     }
   });
 }
+
+/**
+ * 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.
+}
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -356,23 +356,17 @@ file, You can obtain one at http://mozil
                   gBrowser.removeTab(prevTab);
                 return;
               }
             } else if (action.type == "remotetab") {
               url = action.params.url;
             } 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;
             }
             continueOperation.call(this);
           }
           else {
             this._canonizeURL(aTriggeringEvent, response => {
               [url, postData, mayInheritPrincipal] = response;
@@ -450,16 +444,29 @@ file, You can obtain one at http://mozil
               if (matchLastLocationChange) {
                 loadCurrent();
               }
             }
           }
         ]]></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"/>
         <body><![CDATA[
           var url = this.value;
           if (!url) {
             aCallback(["", null, false]);
             return;
@@ -1450,22 +1457,18 @@ file, You can obtain one at http://mozil
               switch (action.type) {
                 case "switchtab": // Fall through.
                 case "keyword": // Fall through.
                 case "visiturl": {
                   url = action.params.url;
                   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: {
                   return;
                 }
               }
             }
 
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -417,49 +417,26 @@ BrowserGlue.prototype = {
         break;
       case "profile-before-change":
          // Any component depending on Places should be finalized in
          // _onPlacesShutdown.  Any component that doesn't need to act after
          // the UI has gone should be finalized in _onQuitApplicationGranted.
         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
         // an engine, and that newly added engines are visible.
         if (data == "engine-added" || data == "engine-removed") {
           let engineName = subject.QueryInterface(Ci.nsISearchEngine).name;
           let Preferences =
             Cu.import("resource://gre/modules/Preferences.jsm", {}).Preferences;