Bug 1659002 - Fix passing region data to notifyObservers. r=daleharvey
authorAichi Chang <phoebexxxc@gmail.com>
Thu, 20 Aug 2020 18:14:05 +0000
changeset 545551 1308c79a573b8846bec73fb2a09967de7dabfb5d
parent 545550 308e4353d2c9ba7f3091449070a0b452914dc10e
child 545552 0d8222cd72dccaa138dfb48f17e5e7e7f81fef2c
push id124651
push userdharvey@mozilla.com
push dateThu, 20 Aug 2020 18:16:49 +0000
treeherderautoland@1308c79a573b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaleharvey
bugs1659002
milestone81.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 1659002 - Fix passing region data to notifyObservers. r=daleharvey Differential Revision: https://phabricator.services.mozilla.com/D87271
toolkit/modules/Region.jsm
toolkit/modules/tests/xpcshell/test_Region.js
--- a/toolkit/modules/Region.jsm
+++ b/toolkit/modules/Region.jsm
@@ -277,16 +277,25 @@ class RegionDetector {
       }
     } else {
       // If we are at home again, stop tracking the seen region.
       prefs.clearUserPref(`${UPDATE_PREFIX}.region`);
       prefs.clearUserPref(`${UPDATE_PREFIX}.first-seen`);
     }
   }
 
+  // Wrap a string as a nsISupports.
+  _createSupportsString(data) {
+    let string = Cc["@mozilla.org/supports-string;1"].createInstance(
+      Ci.nsISupportsString
+    );
+    string.data = data;
+    return string;
+  }
+
   /**
    * Save the updated home region and notify observers.
    *
    * @param {string} region
    *   The region to store.
    * @param {boolean} [notify]
    *   Tests can disable the notification for convenience as it
    *   may trigger an engines reload.
@@ -295,20 +304,19 @@ class RegionDetector {
     if (region == this._home) {
       return;
     }
     log.info("Updating home region:", region);
     this._home = region;
     Services.prefs.setCharPref("browser.search.region", region);
     if (notify) {
       Services.obs.notifyObservers(
-        null,
+        this._createSupportsString(region),
         this.REGION_TOPIC,
-        this.REGION_UPDATED,
-        region
+        this.REGION_UPDATED
       );
     }
   }
 
   /**
    * Make the request to fetch the region from the configured service.
    */
   async _getRegion() {
--- a/toolkit/modules/tests/xpcshell/test_Region.js
+++ b/toolkit/modules/tests/xpcshell/test_Region.js
@@ -15,26 +15,48 @@ const INTERVAL_PREF = "browser.region.up
 
 const RESPONSE_DELAY = 500;
 const RESPONSE_TIMEOUT = 100;
 
 const histogram = Services.telemetry.getHistogramById(
   "SEARCH_SERVICE_COUNTRY_FETCH_RESULT"
 );
 
+// Add notification observer, it will return a promise that will resolve once a notification is fired.
+function waitForNotificationSubject(topic) {
+  return new Promise((resolve, reject) => {
+    Services.obs.addObserver(function observe(aSubject, aTopic, aData) {
+      // wrap the subject as a nsISupports
+      let subject = aSubject.QueryInterface(Ci.nsISupportsString);
+      Services.obs.removeObserver(observe, topic);
+      resolve(subject);
+    }, topic);
+  });
+}
+
 add_task(async function test_basic() {
   let srv = useHttpServer(REGION_PREF);
   srv.registerPathHandler("/", (req, res) => {
     res.setStatusLine("1.1", 200, "OK");
     send(res, { country_code: "UK" });
   });
+  // start to listen the notification
+  let notificationSubjectPromise = await waitForNotificationSubject(
+    "browser-region"
+  );
+  await Region._fetchRegion();
+  let notificationSub = await notificationSubjectPromise;
 
-  await Region._fetchRegion();
   Assert.ok(true, "Region fetch should succeed");
   Assert.equal(Region.home, "UK", "Region fetch should return correct result");
+  Assert.equal(
+    notificationSub,
+    Region.home,
+    "Notification should be sent with the correct region"
+  );
 
   await new Promise(r => srv.stop(r));
 });
 
 add_task(async function test_invalid_url() {
   histogram.clear();
   Services.prefs.setCharPref(REGION_PREF, "http://localhost:0");
   let result = await Region._fetchRegion();