Bug 1462010 - Remove unused migrateRegionPrefs and isUS preference. r?adw draft
authorMichael Kaply <mozilla@kaply.com>
Fri, 18 May 2018 10:10:28 -0500
changeset 796992 62c6af1e2a47c7662882b3521c3d2ca3c2625df7
parent 796991 e1a22438592cb64d35a4a3506d010451e8fd612a
child 797020 66221c97a3baf3415f4bf7e899d38df679f9416b
child 797039 b065e85515604cd640c188ef2b20abb42456032c
push id110393
push usermozilla@kaply.com
push dateFri, 18 May 2018 15:10:47 +0000
reviewersadw
bugs1462010
milestone62.0a1
Bug 1462010 - Remove unused migrateRegionPrefs and isUS preference. r?adw MozReview-Commit-ID: BkwiIUcZW7m
browser/components/search/test/google_codes/browser.ini
testing/profiles/common/user.js
toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_location.js
toolkit/components/search/tests/xpcshell/test_location_malformed_json.js
toolkit/components/search/tests/xpcshell/test_location_migrate_countrycode_isUS.js
toolkit/components/search/tests/xpcshell/test_location_migrate_no_countrycode_isUS.js
toolkit/components/search/tests/xpcshell/test_location_migrate_no_countrycode_notUS.js
toolkit/components/search/tests/xpcshell/test_location_sync.js
toolkit/components/search/tests/xpcshell/test_location_timeout.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/browser/components/search/test/google_codes/browser.ini
+++ b/browser/components/search/test/google_codes/browser.ini
@@ -1,8 +1,9 @@
 [DEFAULT]
 prefs =
   browser.search.countryCode='DE'
+  browser.search.region='DE'
 
 [../browser_google.js]
 skip-if = artifact # bug 1315953
 [../browser_google_behavior.js]
 skip-if = artifact # bug 1315953
--- a/testing/profiles/common/user.js
+++ b/testing/profiles/common/user.js
@@ -1,22 +1,22 @@
 // Base preferences file used by both unittest and perf harnesses.
 /* globals user_pref */
 user_pref("app.update.enabled", false);
 user_pref("browser.dom.window.dump.enabled", true);
 // Use an empty list of sites to avoid fetching
 user_pref("browser.newtabpage.activity-stream.feeds.section.topstories", false);
 user_pref("browser.newtabpage.activity-stream.feeds.snippets", false);
 user_pref("browser.newtabpage.activity-stream.tippyTop.service.endpoint", "");
+// Tell the search service we are running in the US.  This also has the desired
+// side-effect of preventing our geoip lookup.
 user_pref("browser.search.countryCode", "US");
+user_pref("browser.search.region", "US");
 // This will prevent HTTP requests for region defaults.
 user_pref("browser.search.geoSpecificDefaults", false);
-// Tell the search service we are running in the US.  This also has the desired
-// side-effect of preventing our geoip lookup.
-user_pref("browser.search.isUS", true);
 // Disable android snippets
 user_pref("browser.snippets.enabled", false);
 user_pref("browser.snippets.syncPromo.enabled", false);
 // Disable webapp updates.  Yes, it is supposed to be an integer.
 user_pref("browser.webapps.checkForUpdates", 0);
 // We do not wish to display datareporting policy notifications as it might
 // cause other tests to fail. Tests that wish to test the notification functionality
 // should explicitly disable this pref.
--- a/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 ChromeUtils.import("resource://gre/modules/PlacesSearchAutocompleteProvider.jsm");
 
 add_task(async function() {
     // Tell the search service we are running in the US.  This also has the
     // desired side-effect of preventing our geoip lookup.
-   Services.prefs.setBoolPref("browser.search.isUS", true);
    Services.prefs.setCharPref("browser.search.countryCode", "US");
    Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false);
 
    Services.search.restoreDefaultEngines();
    Services.search.resetToOriginalDefaultEngine();
 });
 
 add_task(async function search_engine_match() {
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -384,48 +384,16 @@ function geoSpecificDefaultsEnabled() {
 //   case - 34/35 users who have previously been configured to use US defaults
 //   based purely on a timezone check will have "region" forced to US,
 //   regardless of what countryCode geoip returns.
 // * We may want to know if we are in the US before we have *either*
 //   countryCode or region - in which case we fallback to a timezone check,
 //   but we don't persist that value anywhere in the expectation we will
 //   eventually get a countryCode/region.
 
-// A method that "migrates" prefs if necessary.
-function migrateRegionPrefs() {
-  // If we already have a "region" pref there's nothing to do.
-  if (Services.prefs.prefHasUserValue("browser.search.region")) {
-    return;
-  }
-
-  // If we have 'isUS' but no 'countryCode' then we are almost certainly
-  // a profile from Fx 34/35 that set 'isUS' based purely on a timezone
-  // check. If this said they were US, we force region to be US.
-  // (But if isUS was false, we leave region alone - we will do a geoip request
-  // and set the region accordingly)
-  try {
-    if (Services.prefs.getBoolPref("browser.search.isUS") &&
-        !Services.prefs.prefHasUserValue("browser.search.countryCode")) {
-      Services.prefs.setCharPref("browser.search.region", "US");
-    }
-  } catch (ex) {
-    // no isUS pref, nothing to do.
-  }
-  // If we have a countryCode pref but no region pref, just force region
-  // to be the countryCode.
-  try {
-    let countryCode = Services.prefs.getCharPref("browser.search.countryCode");
-    if (!Services.prefs.prefHasUserValue("browser.search.region")) {
-      Services.prefs.setCharPref("browser.search.region", countryCode);
-    }
-  } catch (ex) {
-    // no countryCode pref, nothing to do.
-  }
-}
-
 // A method to determine if we are in the United States (US) for the search
 // service.
 // It uses a browser.search.region pref (which typically comes from a geoip
 // request) or if that doesn't exist, falls back to a hacky timezone check.
 function getIsUS() {
   // Regardless of the region or countryCode, non en-US builds are not
   // considered to be in the US from the POV of the search service.
   if (getLocale() != "en-US") {
@@ -2672,17 +2640,16 @@ SearchService.prototype = {
   },
 
   // Synchronous implementation of the initializer.
   // Used by |_ensureInitialized| as a fallback if initialization is not
   // complete.
   _syncInit: function SRCH_SVC__syncInit() {
     LOG("_syncInit start");
     this._initStarted = true;
-    migrateRegionPrefs();
 
     let cache = this._readCacheFile();
     if (cache.metaData)
       this._metaData = cache.metaData;
 
     try {
       this._syncLoadEngines(cache);
     } catch (ex) {
@@ -2707,18 +2674,16 @@ SearchService.prototype = {
    * Asynchronous implementation of the initializer.
    *
    * @returns {Promise} A promise, resolved successfully if the initialization
    * succeeds.
    */
   async _asyncInit() {
     LOG("_asyncInit start");
 
-    migrateRegionPrefs();
-
     // 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
     // if a sync init has been forced.
     let cache = await this._asyncReadCacheFile();
 
     try {
       await checkForSyncCompletion(ensureKnownCountryCode(this));
--- a/toolkit/components/search/tests/xpcshell/test_location.js
+++ b/toolkit/components/search/tests/xpcshell/test_location.js
@@ -1,18 +1,16 @@
 /* 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.");
-    // No isUS pref is ever written
-    ok(!Services.prefs.prefHasUserValue("browser.search.isUS"), "no isUS pref");
     // 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_malformed_json.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_malformed_json.js
@@ -21,23 +21,21 @@ function run_test() {
   // setup a console listener for the timezone fallback message.
   let promiseTzMessage = promiseTimezoneMessage();
 
   // 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");
-    ok(!Services.prefs.prefHasUserValue("browser.search.isUS"), "should never be an isUS pref");
     // fetch the engines - this should force the timezone check, but still
     // doesn't 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");
-    ok(!Services.prefs.prefHasUserValue("browser.search.isUS"), "should never be an isUS 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();
       deepEqual(snapshot.counts, [1, 0, 0]); // boolean probe so 3 buckets, expect 1 result for |0|.
deleted file mode 100644
--- a/toolkit/components/search/tests/xpcshell/test_location_migrate_countrycode_isUS.js
+++ /dev/null
@@ -1,22 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Here we are testing the "migration" when both isUS and countryCode are
-// set.
-function run_test() {
-  // Set the prefs we care about.
-  Services.prefs.setBoolPref("browser.search.isUS", true);
-  Services.prefs.setCharPref("browser.search.countryCode", "US");
-  // And the geoip request that will return AU - it shouldn't be used.
-  Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "AU"}');
-  Services.search.init(() => {
-    // "region" and countryCode should still reflect US.
-    equal(Services.prefs.getCharPref("browser.search.region"), "US", "got the correct region.");
-    equal(Services.prefs.getCharPref("browser.search.countryCode"), "US", "got the correct country code.");
-    // should be no geoip evidence.
-    checkCountryResultTelemetry(null);
-    do_test_finished();
-    run_next_test();
-  });
-  do_test_pending();
-}
deleted file mode 100644
--- a/toolkit/components/search/tests/xpcshell/test_location_migrate_no_countrycode_isUS.js
+++ /dev/null
@@ -1,28 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Here we are testing the "migration" from the isUS pref being true but when
-// no country-code exists.
-function run_test() {
-  // Set the pref we care about.
-  Services.prefs.setBoolPref("browser.search.isUS", true);
-  // And the geoip request that will return *not* US.
-  Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "AU"}');
-  Services.search.init(() => {
-    // "region" should be set to US, but countryCode to AU.
-    equal(Services.prefs.getCharPref("browser.search.region"), "US", "got the correct region.");
-    equal(Services.prefs.getCharPref("browser.search.countryCode"), "AU", "got the correct country code.");
-    // 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|.
-    }
-    do_test_finished();
-    run_next_test();
-  });
-  do_test_pending();
-}
deleted file mode 100644
--- a/toolkit/components/search/tests/xpcshell/test_location_migrate_no_countrycode_notUS.js
+++ /dev/null
@@ -1,28 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Here we are testing the "migration" from the isUS pref being false but when
-// no country-code exists.
-function run_test() {
-  // Set the pref we care about.
-  Services.prefs.setBoolPref("browser.search.isUS", false);
-  // And the geoip request that will return US.
-  Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "US"}');
-  Services.search.init(() => {
-    // "region" and countryCode should reflect US.
-    equal(Services.prefs.getCharPref("browser.search.region"), "US", "got the correct region.");
-    equal(Services.prefs.getCharPref("browser.search.countryCode"), "US", "got the correct country code.");
-    // 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|.
-    }
-    do_test_finished();
-    run_next_test();
-  });
-  do_test_pending();
-}
--- a/toolkit/components/search/tests/xpcshell/test_location_sync.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_sync.js
@@ -4,24 +4,16 @@
 function getCountryCodePref() {
   try {
     return Services.prefs.getCharPref("browser.search.countryCode");
   } catch (_) {
     return undefined;
   }
 }
 
-function getIsUSPref() {
-  try {
-    return Services.prefs.getBoolPref("browser.search.isUS");
-  } catch (_) {
-    return undefined;
-  }
-}
-
 // A console listener so we can listen for a log message from nsSearchService.
 function promiseTimezoneMessage() {
   return new Promise(resolve => {
     let listener = {
       QueryInterface: ChromeUtils.generateQI([Ci.nsIConsoleListener]),
       observe(msg) {
         if (msg.message.startsWith("getIsUS() fell back to a timezone check with the result=")) {
           Services.console.unregisterListener(listener);
@@ -32,17 +24,16 @@ function promiseTimezoneMessage() {
     Services.console.registerListener(listener);
   });
 }
 
 // Force a sync init and ensure the right thing happens (ie, that no xhr
 // request is made and we fall back to the timezone-only trick)
 add_task(async function test_simple() {
   deepEqual(getCountryCodePref(), undefined, "no countryCode pref");
-  deepEqual(getIsUSPref(), undefined, "no isUS 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);
 
   // setup a console listener for the timezone fallback message.
   let promiseTzMessage = promiseTimezoneMessage();
--- a/toolkit/components/search/tests/xpcshell/test_location_timeout.js
+++ b/toolkit/components/search/tests/xpcshell/test_location_timeout.js
@@ -58,17 +58,16 @@ function run_test() {
       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");
-      ok(!Services.prefs.prefHasUserValue("browser.search.isUS"), "should never have an isUS pref");
 
       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/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -39,19 +39,16 @@ support-files =
 [test_invalid_engine_from_dir.js]
 [test_init_async_multiple.js]
 [test_init_async_multiple_then_sync.js]
 [test_json_cache.js]
 [test_list_json_searchdefault.js]
 [test_location.js]
 [test_location_error.js]
 [test_location_malformed_json.js]
-[test_location_migrate_countrycode_isUS.js]
-[test_location_migrate_no_countrycode_isUS.js]
-[test_location_migrate_no_countrycode_notUS.js]
 [test_location_partner.js]
 [test_location_funnelcake.js]
 [test_location_sync.js]
 [test_location_timeout.js]
 [test_location_timeout_xhr.js]
 [test_nodb_pluschanges.js]
 [test_save_sorted_engines.js]
 [test_pref.js]