Bug 1184705 - Search A/B testing cohort identifier should be recorded in FHR, r=rnewman.

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

@@ -69,7 +69,7 @@ add_task(function* test_healthreport_search_recording() {
   let oldTelemetry = Services.prefs.getBoolPref("toolkit.telemetry.enabled");
   Services.prefs.setBoolPref("toolkit.telemetry.enabled", true);
 
-  m = provider.getMeasurement("engines", 1);
+  m = provider.getMeasurement("engines", 2);
   yield provider.collectDailyData();
   data = yield m.getValues();
 

services/healthreport/docs/dataformat.rst

@@ -1603,6 +1603,15 @@ default
    In other words, search engines without an ``.identifier``
    are prefixed with ``other-``.
 
+Version 2
+^^^^^^^^^
+
+Starting with Firefox 40, there is an additional optional value:
+
+cohort
+  Daily cohort string identifier, recorded if the user is part of
+  search defaults A/B testing.
+
 org.mozilla.sync.sync
 ---------------------
 

services/healthreport/providers.jsm

@@ -60,6 +60,7 @@ const DAILY_LAST_TEXT_FIELD = {type: Metrics.Storage.FIELD_DAILY_LAST_TEXT};
 const DAILY_COUNTER_FIELD = {type: Metrics.Storage.FIELD_DAILY_COUNTER};
 
 const TELEMETRY_PREF = "toolkit.telemetry.enabled";
+const SEARCH_COHORT_PREF = "browser.search.cohort";
 
 function isTelemetryEnabled(prefs) {
   return prefs.get(TELEMETRY_PREF, false);
@@ -1630,10 +1631,11 @@ SearchEnginesMeasurement1.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "engines",
-  version: 1,
+  version: 2,
 
   fields: {
     default: DAILY_LAST_TEXT_FIELD,
+    cohort: DAILY_LAST_TEXT_FIELD,
   },
 });
 
@@ -1688,6 +1690,9 @@ this.SearchesProvider.prototype = Object.freeze({
       }
 
       yield m.setDailyLastText("default", name);
+
+      if (Services.prefs.prefHasUserValue(SEARCH_COHORT_PREF))
+        yield m.setDailyLastText("cohort", Services.prefs.getCharPref(SEARCH_COHORT_PREF));
     }.bind(this));
   },
 

services/healthreport/tests/xpcshell/test_provider_searches.js

@@ -146,7 +146,7 @@ add_task(function* test_default_search_engine() {
   let provider = new SearchesProvider();
   yield provider.init(storage);
 
-  let m = provider.getMeasurement("engines", 1);
+  let m = provider.getMeasurement("engines", 2);
 
   let now = new Date();
   yield provider.collectDailyData();
@@ -174,5 +174,14 @@ add_task(function* test_default_search_engine() {
   data = yield m.getValues();
   Assert.equal(data.days.getDay(now).get("default"), "other-testdefault");
 
+  // If no cohort identifier is set, we shouldn't report a cohort.
+  Assert.equal(data.days.getDay(now).get("cohort"), undefined);
+
+  // Set a cohort identifier and verify we record it.
+  Services.prefs.setCharPref("browser.search.cohort", "testcohort");
+  yield provider.collectDailyData();
+  data = yield m.getValues();
+  Assert.equal(data.days.getDay(now).get("cohort"), "testcohort");
+
   yield storage.close();
 });

toolkit/components/telemetry/TelemetryEnvironment.jsm

@@ -153,6 +153,7 @@ const PREF_PARTNER_ID = "mozilla.partner.id";
 const PREF_TELEMETRY_ENABLED = "toolkit.telemetry.enabled";
 const PREF_UPDATE_ENABLED = "app.update.enabled";
 const PREF_UPDATE_AUTODOWNLOAD = "app.update.auto";
+const PREF_SEARCH_COHORT = "browser.search.cohort";
 
 const EXPERIMENTS_CHANGED_TOPIC = "experiments-changed";
 const SEARCH_ENGINE_MODIFIED_TOPIC = "browser-search-engine-modified";
@@ -891,6 +892,10 @@ EnvironmentCache.prototype = {
     this._currentEnvironment.settings.defaultSearchEngine = this._getDefaultSearchEngine();
     this._currentEnvironment.settings.defaultSearchEngineData =
       Services.search.getDefaultEngineInfo();
+
+    // Record the cohort identifier used for search defaults A/B testing.
+    if (Services.prefs.prefHasUserValue(PREF_SEARCH_COHORT))
+      this._currentEnvironment.settings.searchCohort = Services.prefs.getCharPref(PREF_SEARCH_COHORT);
   },
 
   /**

toolkit/components/telemetry/docs/environment.rst

@@ -266,3 +266,8 @@ The object contains:
   For privacy, we don't record this for user-installed engines.
 
 ``loadPath`` and ``submissionURL`` are not present if ``name`` is ``NONE``.
+
+searchCohort
+~~~~~~~~~~~~
+
+If the user has been enrolled into a search default change experiment, this contains the string identifying the experiment the user is taking part in. Most user profiles will never be part of any search default change experiment, and will not send this value.

toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js

@@ -1103,6 +1103,15 @@ add_task(function* test_defaultSearchEngine() {
   data = TelemetryEnvironment.currentEnvironment;
   checkEnvironmentData(data);
   Assert.equal(data.settings.defaultSearchEngine, EXPECTED_SEARCH_ENGINE);
+
+  // Check that by default we are not sending a cohort identifier...
+  Assert.equal(data.settings.searchCohort, undefined);
+
+  // ... but that if a cohort identifier is set, we send it.
+  Services.prefs.setCharPref("browser.search.cohort", "testcohort");
+  Services.obs.notifyObservers(null, "browser-search-service", "init-complete");
+  data = TelemetryEnvironment.currentEnvironment;
+  Assert.equal(data.settings.searchCohort, "testcohort");
 });
 
 add_task(function*() {