Bug 1929411 - Add home region telemetry to Glean. r=scunnane

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

toolkit/modules/Region.sys.mjs

@@ -124,7 +124,10 @@ class RegionDetector {
   // Keep track of how many times we have tried to fetch
   // the users region during failure.
   _retryCount = 0;
-  // Let tests wait for init to complete.
+  /**
+   * @type {Promise}
+   *   Allow tests to wait for init to be complete.
+   */
   _initPromise = null;
   // Topic for Observer events fired by Region.sys.mjs.
   REGION_TOPIC = "browser-region-updated";
@@ -140,11 +143,18 @@ class RegionDetector {
    * Read currently stored region data and if needed trigger background
    * region detection.
    */
-  async init() {
+  init() {
+    // If we're running in the child process, then all `Region` does is act
+    // as a proxy for the browser.search.region preference.
+    if (inChildProcess) {
+      this._home = Services.prefs.getCharPref(REGION_PREF, null);
+      return Promise.resolve();
+    }
+
     if (this._initPromise) {
       return this._initPromise;
     }
-    if (lazy.cacheBustEnabled && !inChildProcess) {
+    if (lazy.cacheBustEnabled) {
       Services.tm.idleDispatchToMainThread(() => {
         lazy.timerManager.registerTimer(
           UPDATE_CHECK_NAME,
@@ -155,10 +165,13 @@ class RegionDetector {
     }
     let promises = [];
     this._home = Services.prefs.getCharPref(REGION_PREF, null);
-    if (!this._home && !inChildProcess) {
+    if (this._home) {
+      // On startup, ensure the Glean probe knows the home region from preferences.
+      Glean.region.homeRegion.set(this._home);
+    } else {
       promises.push(this._idleDispatch(() => this._fetchRegion()));
     }
-    if (lazy.localGeocodingEnabled && !inChildProcess) {
+    if (lazy.localGeocodingEnabled) {
       promises.push(this._idleDispatch(() => this._setupRemoteSettings()));
     }
     return (this._initPromise = Promise.all(promises));
@@ -167,7 +180,7 @@ class RegionDetector {
   /**
    * Get the region we currently consider the users home.
    *
-   * @returns {string}
+   * @returns {?string}
    *   The users current home region.
    */
   get home() {
@@ -314,6 +327,7 @@ class RegionDetector {
     log.info("Updating home region:", region);
     this._home = region;
     Services.prefs.setCharPref("browser.search.region", region);
+    Glean.region.homeRegion.set(region);
     if (notify) {
       Services.obs.notifyObservers(
         this._createSupportsString(region),

toolkit/modules/gecko_metrics.yaml

@@ -71,6 +71,30 @@ region:
     no_lint:
       - COMMON_PREFIX
 
+  home_region:
+    type: string
+    lifetime: application
+    description: >
+      Records the detected home region of the user. This is the general region
+      of the user's machine.
+
+      If a machine moves location, there is a minimum 2-week delay before this
+      will be updated.
+
+      See the [Region documentation](https://firefox-source-docs.mozilla.org/toolkit/modules/toolkit_modules/Region.html)
+      for more information about updates.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1419788
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1929411
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1419788
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1929411
+    notification_emails:
+      - fx-search-telemetry@mozilla.com
+    data_sensitivity:
+      - interaction
+    expires: never
+
   store_region_result:
     type: labeled_counter
     description: >

toolkit/modules/tests/xpcshell/test_Region.js

@@ -3,9 +3,6 @@
 const { HttpServer } = ChromeUtils.importESModule(
   "resource://testing-common/httpd.sys.mjs"
 );
-const { Region } = ChromeUtils.importESModule(
-  "resource://gre/modules/Region.sys.mjs"
-);
 const { setTimeout } = ChromeUtils.importESModule(
   "resource://gre/modules/Timer.sys.mjs"
 );
@@ -17,6 +14,7 @@ const { sinon } = ChromeUtils.importESModule(
 );
 
 ChromeUtils.defineESModuleGetters(this, {
+  Region: "resource://gre/modules/Region.sys.mjs",
   RegionTestUtils: "resource://testing-common/RegionTestUtils.sys.mjs",
 });
 
@@ -29,15 +27,52 @@ const histogram = Services.telemetry.getHistogramById(
   "SEARCH_SERVICE_COUNTRY_FETCH_RESULT"
 );
 
+add_setup(async () => {
+  Services.prefs.setBoolPref("browser.region.log", true);
+});
+
 // Region.sys.mjs will call init() on being loaded and set a background
 // task to fetch the region, ensure we have completed this before
 // running the rest of the tests.
-add_task(async function test_startup() {
+add_task(async function test_startup_with_no_pref() {
-  RegionTestUtils.setNetworkRegion("UK");
+  Services.fog.testResetFOG();
+
+  RegionTestUtils.setNetworkRegion("AT");
+
+  // Region.sys.mjs is lazily loaded, so referencing `Region.` here will load
+  // it and automatically call the `Region.init()` function.
   await checkTelemetry(Region.TELEMETRY.SUCCESS);
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "AT",
+    "Should have correctly stored the region in home region after getting it"
+  );
+
   await cleanup();
 });
 
+add_task(async function test_startup_with_pref() {
+  Services.fog.testResetFOG();
+
+  Services.prefs.setCharPref("browser.search.region", "GB");
+
+  // If we failed to read the pref, we'd kick off a network connection. So set
+  // up the network to return a different region here so that we'd be able to
+  // detect that case.
+  RegionTestUtils.setNetworkRegion("DE");
+
+  // Use a different instance of region for testing this.
+  let region = Region.newInstance();
+  await region.init();
+
+  Assert.equal(region.home, "GB", "Should have loaded the correct region");
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "GB",
+    "Should have correctly stored the region in home region after getting it"
+  );
+});
+
 add_task(async function test_basic() {
   Services.fog.testResetFOG();
 
@@ -61,6 +96,12 @@ add_task(async function test_basic() {
 
   assertStoredResultTelemetry({ restOfWorld: 1 });
 
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "UK",
+    "Should have correctly set the region in home region after getting it"
+  );
+
   await cleanup(srv);
 });
 
@@ -123,23 +164,45 @@ add_task(async function test_location() {
 });
 
 add_task(async function test_update() {
-  Region._home = null;
   RegionTestUtils.setNetworkRegion("FR");
   await Region._fetchRegion();
   Assert.equal(Region.home, "FR", "Should have correct region");
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "FR",
+    "Should have correctly set the region in home region after getting it"
+  );
+
   RegionTestUtils.setNetworkRegion("DE");
   await Region._fetchRegion();
   Assert.equal(Region.home, "FR", "Shouldnt have changed yet");
-  // Thie first fetchRegion will set the prefs to determine when
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "FR",
+    "Should not have updated the home region telemetry yet"
+  );
+
+  // The first fetchRegion will set the prefs to determine when
   // to update the home region, we need to do 2 fetchRegions to test
   // it isnt updating when it shouldnt.
   await Region._fetchRegion();
   Assert.equal(Region.home, "FR", "Shouldnt have changed yet again");
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "FR",
+    "Should not have updated the home region telemetry yet (again)"
+  );
+
   Services.prefs.setIntPref(INTERVAL_PREF, 1);
   /* eslint-disable mozilla/no-arbitrary-setTimeout */
   await new Promise(resolve => setTimeout(resolve, 1100));
   await Region._fetchRegion();
   Assert.equal(Region.home, "DE", "Should have updated now");
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "DE",
+    "Should have correctly set the home region telemetry"
+  );
 
   await cleanup();
 });
@@ -155,6 +218,11 @@ add_task(async function test_update_us() {
 
   assertStoredResultTelemetry({ unitedStates: 1 });
   Assert.equal(Region.home, "US", "Should have correct region");
+  Assert.equal(
+    Glean.region.homeRegion.testGetValue(),
+    "US",
+    "Should have correctly set the home region telemetry"
+  );
 
   Services.fog.testResetFOG();
 
@@ -268,6 +336,7 @@ function send(res, json) {
 
 async function cleanup(srv = null) {
   Services.prefs.clearUserPref("browser.search.region");
+  Region._home = null;
   if (srv) {
     await new Promise(r => srv.stop(r));
   }
@@ -277,7 +346,7 @@ async function checkTelemetry(aExpectedValue) {
   // Wait until there is 1 result.
   await TestUtils.waitForCondition(() => {
     let snapshot = histogram.snapshot();
-    return Object.values(snapshot.values).reduce((a, b) => a + b) == 1;
+    return Object.values(snapshot.values).reduce((a, b) => a + b, 0) == 1;
   });
   let snapshot = histogram.snapshot();
   Assert.equal(snapshot.values[aExpectedValue], 1);