Bug 1462015 - Remove browser.search.countryCode pref. r?florian
MozReview-Commit-ID: J5IdlFfDlXd
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -390,26 +390,26 @@ function isUSTimezone() {
// 600 minutes = 10 hours (UTC-10), which is
// Hawaii-Aleutian Standard Time (http://www.timeanddate.com/time/zones/hast)
let UTCOffset = (new Date()).getTimezoneOffset();
return UTCOffset >= 150 && UTCOffset <= 600;
}
-// A method that tries to determine our country-code via an XHR geoip lookup.
-var ensureKnownCountryCode = async function(ss) {
- // If we have a country-code already stored in our prefs we trust it.
- let countryCode = Services.prefs.getCharPref("browser.search.countryCode", "");
-
- if (!countryCode) {
- // We don't have it cached, so fetch it. fetchCountryCode() will call
- // storeCountryCode if it gets a result (even if that happens after the
+// A method that tries to determine our region via an XHR geoip lookup.
+var ensureKnownRegion = async function(ss) {
+ // If we have a region already stored in our prefs we trust it.
+ let region = Services.prefs.getCharPref("browser.search.region", "");
+
+ if (!region) {
+ // We don't have it cached, so fetch it. fetchREgion() will call
+ // storeRegion if it gets a result (even if that happens after the
// promise resolves) and fetchRegionDefault.
- await fetchCountryCode(ss);
+ await fetchRegion(ss);
} else {
// if nothing to do, return early.
if (!geoSpecificDefaultsEnabled())
return;
let expir = ss.getGlobalAttr("searchDefaultExpir") || 0;
if (expir > Date.now()) {
// The territory default we have already fetched hasn't expired yet.
@@ -443,37 +443,26 @@ var ensureKnownCountryCode = async funct
});
}
// If gInitialized is true then the search service was forced to perform
// a sync initialization during our XHRs - capture this via telemetry.
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT").add(gInitialized);
};
-// Some notes on countryCode and region prefs:
-// * A "countryCode" pref is set via a geoip lookup. It always reflects the
-// result of that geoip request.
-// * A "region" pref, once set, is the region actually used for search. In
-// most cases it will be identical to the countryCode pref.
-
// Store the result of the geoip request as well as any other values and
// telemetry which depend on it.
-function storeCountryCode(cc) {
- // Set the country-code itself.
- Services.prefs.setCharPref("browser.search.countryCode", cc);
- // And set the region pref if we don't already have a value.
- if (!Services.prefs.prefHasUserValue("browser.search.region")) {
- Services.prefs.setCharPref("browser.search.region", cc);
- }
+function storeRegion(region) {
+ Services.prefs.setCharPref("browser.search.region", region);
// and telemetry...
let isTimezoneUS = isUSTimezone();
- if (cc == "US" && !isTimezoneUS) {
+ if (region == "US" && !isTimezoneUS) {
Services.telemetry.getHistogramById("SEARCH_SERVICE_US_COUNTRY_MISMATCHED_TIMEZONE").add(1);
}
- if (cc != "US" && isTimezoneUS) {
+ if (region != "US" && isTimezoneUS) {
Services.telemetry.getHistogramById("SEARCH_SERVICE_US_TIMEZONE_MISMATCHED_COUNTRY").add(1);
}
// telemetry to compare our geoip response with platform-specific country data.
// On Mac and Windows, we can get a country code via sysinfo
let platformCC = Services.sysinfo.get("countryCode");
if (platformCC) {
let probeUSMismatched, probeNonUSMismatched;
switch (Services.appinfo.OS) {
@@ -485,41 +474,41 @@ function storeCountryCode(cc) {
probeUSMismatched = "SEARCH_SERVICE_US_COUNTRY_MISMATCHED_PLATFORM_WIN";
probeNonUSMismatched = "SEARCH_SERVICE_NONUS_COUNTRY_MISMATCHED_PLATFORM_WIN";
break;
default:
Cu.reportError("Platform " + Services.appinfo.OS + " has system country code but no search service telemetry probes");
break;
}
if (probeUSMismatched && probeNonUSMismatched) {
- if (cc == "US" || platformCC == "US") {
+ if (region == "US" || platformCC == "US") {
// one of the 2 said US, so record if they are the same.
- Services.telemetry.getHistogramById(probeUSMismatched).add(cc != platformCC);
+ Services.telemetry.getHistogramById(probeUSMismatched).add(region != platformCC);
} else {
- // different country - record if they are the same
- Services.telemetry.getHistogramById(probeNonUSMismatched).add(cc != platformCC);
+ // non-US - record if they are the same
+ Services.telemetry.getHistogramById(probeNonUSMismatched).add(region != platformCC);
}
}
}
}
-// Get the country we are in via a XHR geoip request.
-function fetchCountryCode(ss) {
+// Get the region we are in via a XHR geoip request.
+function fetchRegion(ss) {
// values for the SEARCH_SERVICE_COUNTRY_FETCH_RESULT 'enum' telemetry probe.
const TELEMETRY_RESULT_ENUM = {
SUCCESS: 0,
SUCCESS_WITHOUT_DATA: 1,
XHRTIMEOUT: 2,
ERROR: 3,
// Note that we expect to add finer-grained error types here later (eg,
// dns error, network error, ssl error, etc) with .ERROR remaining as the
// generic catch-all that doesn't fit into other categories.
};
let endpoint = Services.urlFormatter.formatURLPref("browser.search.geoip.url");
- LOG("_fetchCountryCode starting with endpoint " + endpoint);
+ LOG("_fetchRegion starting with endpoint " + endpoint);
// As an escape hatch, no endpoint means no geoip.
if (!endpoint) {
return Promise.resolve();
}
let startTime = Date.now();
return new Promise(resolve => {
// Instead of using a timeout on the xhr object itself, we simulate one
// using a timer and let the XHR request complete. This allows us to
@@ -528,28 +517,28 @@ function fetchCountryCode(ss) {
// that many users end up doing a sync init of the search service and thus
// would see the jank that implies.
// (Note we do actually use a timeout on the XHR, but that's set to be a
// large value just incase the request never completes - we don't want the
// XHR object to live forever)
let timeoutMS = Services.prefs.getIntPref("browser.search.geoip.timeout");
let geoipTimeoutPossible = true;
let timerId = setTimeout(() => {
- LOG("_fetchCountryCode: timeout fetching country information");
+ LOG("_fetchRegion: timeout fetching region information");
if (geoipTimeoutPossible)
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_TIMEOUT").add(1);
timerId = null;
resolve();
}, timeoutMS);
let resolveAndReportSuccess = (result, reason) => {
- // Even if we timed out, we want to save the country code and everything
+ // Even if we timed out, we want to save the region and everything
// related so next startup sees the value and doesn't retry this dance.
if (result) {
- storeCountryCode(result);
+ storeRegion(result);
}
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_FETCH_RESULT").add(reason);
// This notification is just for tests...
Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "geoip-lookup-xhr-complete");
if (timerId) {
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_TIMEOUT").add(0);
@@ -577,28 +566,28 @@ function fetchCountryCode(ss) {
};
let request = new XMLHttpRequest();
// This notification is just for tests...
Services.obs.notifyObservers(request, SEARCH_SERVICE_TOPIC, "geoip-lookup-xhr-starting");
request.timeout = 100000; // 100 seconds as the last-chance fallback
request.onload = function(event) {
let took = Date.now() - startTime;
- let cc = event.target.response && event.target.response.country_code;
- LOG("_fetchCountryCode got success response in " + took + "ms: " + cc);
+ let region = event.target.response && event.target.response.country_code;
+ LOG("_fetchRegion got success response in " + took + "ms: " + region);
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_FETCH_TIME_MS").add(took);
- let reason = cc ? TELEMETRY_RESULT_ENUM.SUCCESS : TELEMETRY_RESULT_ENUM.SUCCESS_WITHOUT_DATA;
- resolveAndReportSuccess(cc, reason);
+ let reason = region ? TELEMETRY_RESULT_ENUM.SUCCESS : TELEMETRY_RESULT_ENUM.SUCCESS_WITHOUT_DATA;
+ resolveAndReportSuccess(region, reason);
};
request.ontimeout = function(event) {
- LOG("_fetchCountryCode: XHR finally timed-out fetching country information");
+ LOG("_fetchRegion: XHR finally timed-out fetching region information");
resolveAndReportSuccess(null, TELEMETRY_RESULT_ENUM.XHRTIMEOUT);
};
request.onerror = function(event) {
- LOG("_fetchCountryCode: failed to retrieve country information");
+ LOG("_fetchRegion: failed to retrieve region information");
resolveAndReportSuccess(null, TELEMETRY_RESULT_ENUM.ERROR);
};
request.open("POST", endpoint, true);
request.setRequestHeader("Content-Type", "application/json");
request.responseType = "json";
request.send("{}");
});
}
@@ -2635,27 +2624,27 @@ SearchService.prototype = {
* @returns {Promise} A promise, resolved successfully if the initialization
* succeeds.
*/
async _asyncInit() {
LOG("_asyncInit start");
// See if we have a cache file so we don't have to parse a bunch of XML.
// Not using checkForSyncCompletion here because we want to ensure we
- // fetch the country code and geo specific defaults asynchronously even
+ // fetch the region and geo specific defaults asynchronously even
// if a sync init has been forced.
let cache = await this._asyncReadCacheFile();
try {
- await checkForSyncCompletion(ensureKnownCountryCode(this));
+ await checkForSyncCompletion(ensureKnownRegion(this));
} catch (ex) {
if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
throw ex;
}
- LOG("_asyncInit: failure determining country code: " + ex);
+ LOG("_asyncInit: failure determining region: " + ex);
}
try {
await checkForSyncCompletion(this._asyncLoadEngines(cache));
} catch (ex) {
if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
throw ex;
}
this._initRV = Cr.NS_ERROR_FAILURE;
@@ -2949,18 +2938,18 @@ SearchService.prototype = {
// Tests that want to force a synchronous re-initialization need to
// be notified when we are done uninitializing.
Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC,
"uninit-complete");
let cache = await this._asyncReadCacheFile();
- await ensureKnownCountryCode(this);
- // Due to the HTTP requests done by ensureKnownCountryCode, it's possible that
+ await ensureKnownRegion(this);
+ // Due to the HTTP requests done by ensureKnownRegion, it's possible that
// at this point a synchronous init has been forced by other code.
if (!gInitialized)
await this._asyncLoadEngines(cache);
// Typically we'll re-init as a result of a pref observer,
// so signal to 'callers' that we're done.
gInitialized = true;
Services.obs.notifyObservers(null, SEARCH_SERVICE_TOPIC, "init-complete");
@@ -3379,17 +3368,17 @@ SearchService.prototype = {
if (!("default" in json)) {
Cu.reportError("parseListJSON: Missing default in list.json");
dump("parseListJSON: Missing default in list.json\n");
return;
}
searchSettings = json;
}
- // Check if we have a useable country specific list of visible default engines.
+ // Check if we have a useable region specific list of visible default engines.
// This will only be set if we got the list from the Mozilla search server;
// it will not be set for distributions.
let engineNames;
let visibleDefaultEngines = this.getVerifiedGlobalAttr("visibleDefaultEngines");
if (visibleDefaultEngines) {
let jarNames = new Set();
for (let region in searchSettings) {
// Artifact builds use the full list.json which parses
@@ -3400,17 +3389,17 @@ SearchService.prototype = {
for (let engine of searchSettings[region].visibleDefaultEngines) {
jarNames.add(engine);
}
}
engineNames = visibleDefaultEngines.split(",");
for (let engineName of engineNames) {
// If all engineName values are part of jarNames,
- // then we can use the country specific list, otherwise ignore it.
+ // then we can use the region specific list, otherwise ignore it.
// The visibleDefaultEngines string containing the name of an engine we
// don't ship indicates the server is misconfigured to answer requests
// from the specific Firefox version we are running, so ignoring the
// value altogether is safer.
if (!jarNames.has(engineName)) {
LOG("_parseListJSON: ignoring visibleDefaultEngines value because " +
engineName + " is not in the jar engines we have found");
engineNames = null;
--- a/toolkit/components/search/tests/xpcshell/test_geodefaults.js
+++ b/toolkit/components/search/tests/xpcshell/test_geodefaults.js
@@ -80,19 +80,19 @@ add_task(async function no_request_if_pr
Assert.equal(typeof metadata.searchDefaultHash, "undefined");
Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", true);
});
add_task(async function should_get_geo_defaults_only_once() {
// (Re)initializing the search service should trigger a request,
// and set the default engine based on it.
- // Due to the previous initialization, we expect the countryCode to already be set.
- Assert.ok(Services.prefs.prefHasUserValue("browser.search.countryCode"));
- Assert.equal(Services.prefs.getCharPref("browser.search.countryCode"), "FR");
+ // Due to the previous initialization, we expect the region to already be set.
+ Assert.ok(Services.prefs.prefHasUserValue("browser.search.region"));
+ Assert.equal(Services.prefs.getCharPref("browser.search.region"), "FR");
await asyncReInit();
checkRequest();
Assert.equal(Services.search.currentEngine.name, kTestEngineName);
await promiseAfterCache();
// Verify the metadata was written correctly.
let metadata = await promiseGlobalMetadata();
Assert.equal(typeof metadata.searchDefaultExpir, "number");
@@ -103,18 +103,18 @@ add_task(async function should_get_geo_d
Assert.equal(metadata.searchDefaultHash.length, 44);
// The next restart shouldn't trigger a request.
await asyncReInit();
checkNoRequest();
Assert.equal(Services.search.currentEngine.name, kTestEngineName);
});
-add_task(async function should_request_when_countryCode_not_set() {
- Services.prefs.clearUserPref("browser.search.countryCode");
+add_task(async function should_request_when_region_not_set() {
+ Services.prefs.clearUserPref("browser.search.region");
await asyncReInit();
checkRequest();
await promiseAfterCache();
});
add_task(async function should_recheck_if_interval_expired() {
await forceExpiration();
--- a/toolkit/components/search/tests/xpcshell/test_location.js
+++ b/toolkit/components/search/tests/xpcshell/test_location.js
@@ -1,16 +1,15 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
function run_test() {
Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "AU"}');
Services.search.init(() => {
- equal(Services.prefs.getCharPref("browser.search.countryCode"), "AU", "got the correct country code.");
- equal(Services.prefs.getCharPref("browser.search.region"), "AU", "region pref also set to the countryCode.");
+ equal(Services.prefs.getCharPref("browser.search.region"), "AU", "got the correct region.");
// check we have "success" recorded in telemetry
checkCountryResultTelemetry(TELEMETRY_RESULT_ENUM.SUCCESS);
// a false value for each of SEARCH_SERVICE_COUNTRY_TIMEOUT and SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT
for (let hid of ["SEARCH_SERVICE_COUNTRY_TIMEOUT",
"SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT"]) {
let histogram = Services.telemetry.getHistogramById(hid);
let snapshot = histogram.snapshot();
deepEqual(snapshot.counts, [1, 0, 0]); // boolean probe so 3 buckets, expect 1 result for |0|.
--- a/toolkit/components/search/tests/xpcshell/test_location_error.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_error.js
@@ -3,18 +3,18 @@
function run_test() {
// We use an invalid port that parses but won't open
let url = "http://localhost:0";
Services.prefs.setCharPref("browser.search.geoip.url", url);
Services.search.init(() => {
try {
- Services.prefs.getCharPref("browser.search.countryCode");
- ok(false, "not expecting countryCode to be set");
+ Services.prefs.getCharPref("browser.search.region");
+ ok(false, "not expecting region to be set");
} catch (ex) {}
// should have an error recorded.
checkCountryResultTelemetry(TELEMETRY_RESULT_ENUM.ERROR);
// but false values for timeout and forced-sync-init.
for (let hid of ["SEARCH_SERVICE_COUNTRY_TIMEOUT",
"SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT"]) {
let histogram = Services.telemetry.getHistogramById(hid);
let snapshot = histogram.snapshot();
--- a/toolkit/components/search/tests/xpcshell/test_location_malformed_json.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_malformed_json.js
@@ -16,21 +16,19 @@ function promiseTimezoneMessage() {
Services.console.registerListener(listener);
});
}
function run_test() {
// Here we have malformed JSON
Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code"');
Services.search.init(() => {
- ok(!Services.prefs.prefHasUserValue("browser.search.countryCode"), "should be no countryCode pref");
ok(!Services.prefs.prefHasUserValue("browser.search.region"), "should be no region pref");
// fetch the engines - this should not persist any prefs.
Services.search.getEngines();
- ok(!Services.prefs.prefHasUserValue("browser.search.countryCode"), "should be no countryCode pref");
ok(!Services.prefs.prefHasUserValue("browser.search.region"), "should be no region pref");
// should have recorded SUCCESS_WITHOUT_DATA
checkCountryResultTelemetry(TELEMETRY_RESULT_ENUM.SUCCESS_WITHOUT_DATA);
// and false values for timeout and forced-sync-init.
for (let hid of ["SEARCH_SERVICE_COUNTRY_TIMEOUT",
"SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT"]) {
let histogram = Services.telemetry.getHistogramById(hid);
let snapshot = histogram.snapshot();
--- a/toolkit/components/search/tests/xpcshell/test_location_sync.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_sync.js
@@ -1,39 +1,39 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
-function getCountryCodePref() {
+function getRegionPref() {
try {
- return Services.prefs.getCharPref("browser.search.countryCode");
+ return Services.prefs.getCharPref("browser.search.region");
} catch (_) {
return undefined;
}
}
// Force a sync init and ensure the right thing happens (ie, that no xhr
// request is made )
add_task(async function test_simple() {
- deepEqual(getCountryCodePref(), undefined, "no countryCode pref");
+ deepEqual(getRegionPref(), undefined, "no region pref");
// Still set a geoip pref so we can (indirectly) check it wasn't used.
Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "AU"}');
ok(!Services.search.isInitialized);
// fetching the engines forces a sync init
Services.search.getEngines();
ok(Services.search.isInitialized);
// a little wait to check we didn't do the xhr thang.
await new Promise(resolve => {
do_timeout(500, resolve);
});
- deepEqual(getCountryCodePref(), undefined, "didn't do the geoip xhr");
+ deepEqual(getRegionPref(), undefined, "didn't do the geoip xhr");
// and no telemetry evidence of geoip.
for (let hid of [
"SEARCH_SERVICE_COUNTRY_FETCH_RESULT",
"SEARCH_SERVICE_COUNTRY_FETCH_TIME_MS",
"SEARCH_SERVICE_COUNTRY_TIMEOUT",
"SEARCH_SERVICE_US_COUNTRY_MISMATCHED_TIMEZONE",
"SEARCH_SERVICE_US_TIMEZONE_MISMATCHED_COUNTRY",
"SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT",
--- a/toolkit/components/search/tests/xpcshell/test_location_timeout.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_timeout.js
@@ -33,17 +33,16 @@ function run_test() {
resolveContinuePromise = resolve;
});
let server = startServer(continuePromise);
let url = "http://localhost:" + server.identity.primaryPort + "/lookup_country";
Services.prefs.setCharPref("browser.search.geoip.url", url);
Services.prefs.setIntPref("browser.search.geoip.timeout", 50);
Services.search.init(() => {
- ok(!Services.prefs.prefHasUserValue("browser.search.countryCode"), "should be no countryCode pref");
ok(!Services.prefs.prefHasUserValue("browser.search.region"), "should be no region pref");
// should be no result recorded at all.
checkCountryResultTelemetry(null);
// should have set the flag indicating we saw a timeout.
let histogram = Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_TIMEOUT");
let snapshot = histogram.snapshot();
deepEqual(snapshot.counts, [0, 1, 0]);
@@ -56,17 +55,16 @@ function run_test() {
// it timed out.
// The telemetry "sum" will be the actual time in ms - just check it's non-zero.
ok(getProbeSum("SEARCH_SERVICE_COUNTRY_FETCH_TIME_MS") != 0);
// should have reported the fetch ended up being successful
checkCountryResultTelemetry(TELEMETRY_RESULT_ENUM.SUCCESS);
// and should have the result of the response that finally came in, and
// everything dependent should also be updated.
- equal(Services.prefs.getCharPref("browser.search.countryCode"), "AU");
equal(Services.prefs.getCharPref("browser.search.region"), "AU");
do_test_finished();
server.stop(run_next_test);
});
// now tell the server to send its response. That will end up causing the
// search service to notify of that the response was received.
resolveContinuePromise();
--- a/toolkit/components/search/tests/xpcshell/test_location_timeout_xhr.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_timeout_xhr.js
@@ -37,17 +37,16 @@ function run_test() {
let server = startServer(continuePromise);
let url = "http://localhost:" + server.identity.primaryPort + "/lookup_country";
Services.prefs.setCharPref("browser.search.geoip.url", url);
// The timeout for the timer.
Services.prefs.setIntPref("browser.search.geoip.timeout", 10);
let promiseXHRStarted = waitForSearchNotification("geoip-lookup-xhr-starting");
Services.search.init(() => {
- ok(!Services.prefs.prefHasUserValue("browser.search.countryCode"), "should be no countryCode pref");
ok(!Services.prefs.prefHasUserValue("browser.search.region"), "should be no region pref");
// should be no result recorded at all.
checkCountryResultTelemetry(null);
// should have set the flag indicating we saw a timeout.
let histogram = Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_TIMEOUT");
let snapshot = histogram.snapshot();
deepEqual(snapshot.counts, [0, 1, 0]);
@@ -63,17 +62,16 @@ function run_test() {
// wait for the xhr timeout to fire.
waitForSearchNotification("geoip-lookup-xhr-complete").then(() => {
// should have the XHR timeout recorded.
checkCountryResultTelemetry(TELEMETRY_RESULT_ENUM.XHRTIMEOUT);
// still should not have a report of how long the response took as we
// only record that on success responses.
verifyProbeSum("SEARCH_SERVICE_COUNTRY_FETCH_TIME_MS", 0);
// and we still don't know the country code or region.
- ok(!Services.prefs.prefHasUserValue("browser.search.countryCode"), "should be no countryCode pref");
ok(!Services.prefs.prefHasUserValue("browser.search.region"), "should be no region pref");
// unblock the server even though nothing is listening.
resolveContinuePromise();
do_test_finished();
server.stop(run_next_test);
});