Bug 1369778 - Reset search expiration on major updates. r=florian
authorMichael Kaply <mozilla@kaply.com>
Thu, 28 Sep 2017 14:32:47 -0500
changeset 384689 0b50187d11f9299a66c2ea797943dcbb2a1ab2aa
parent 384688 e16347aa774bae92a677bf484d84a5a5c0c60152
child 384690 62fc2f51d566974bf2de7b93cf4588a2c2e81c4d
push id95823
push usermozilla@kaply.com
push dateThu, 05 Oct 2017 13:17:03 +0000
treeherdermozilla-inbound@0b50187d11f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1369778
milestone58.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 1369778 - Reset search expiration on major updates. r=florian MozReview-Commit-ID: Jcr2dx9Bbza
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_async_migration.js
toolkit/components/search/tests/xpcshell/test_geodefaults.js
toolkit/components/search/tests/xpcshell/test_sync_migration.js
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -2720,24 +2720,20 @@ SearchService.prototype = {
    * 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.
-    let cache = {};
     // 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.
-    cache = await this._asyncReadCacheFile();
-
-    if (!gInitialized && cache.metaData)
-      this._metaData = cache.metaData;
+    let cache = await this._asyncReadCacheFile();
 
     try {
       await checkForSyncCompletion(ensureKnownCountryCode(this));
     } catch (ex) {
       if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
         throw ex;
       }
       LOG("_asyncInit: failure determining country code: " + ex);
@@ -2820,25 +2816,28 @@ SearchService.prototype = {
 
   _buildCache: function SRCH_SVC__buildCache() {
     if (this._batchTask)
       this._batchTask.disarm();
 
     let cache = {};
     let locale = getLocale();
     let buildID = Services.appinfo.platformBuildID;
+    let appVersion = Services.appinfo.version;
 
     // Allows us to force a cache refresh should the cache format change.
     cache.version = CACHE_VERSION;
     // We don't want to incur the costs of stat()ing each plugin on every
     // startup when the only (supported) time they will change is during
     // app updates (where the buildID is obviously going to change).
     // Extension-shipped plugins are the only exception to this, but their
     // directories are blown away during updates, so we'll detect their changes.
     cache.buildID = buildID;
+    // Store the appVersion as well so we can do extra things during major updates.
+    cache.appVersion = appVersion;
     cache.locale = locale;
 
     cache.visibleDefaultEngines = this._visibleDefaultEngines;
     cache.metaData = this._metaData;
     cache.engines = [];
 
     for (let name in this._engines) {
       cache.engines.push(this._engines[name]);
@@ -3103,20 +3102,17 @@ SearchService.prototype = {
         this._metaData = {};
         this._cacheFileJSON = null;
 
         // 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 = {};
-        cache = await this._asyncReadCacheFile();
-        if (!gInitialized && cache.metaData)
-          this._metaData = cache.metaData;
+        let cache = await this._asyncReadCacheFile();
 
         await ensureKnownCountryCode(this);
         // Due to the HTTP requests done by ensureKnownCountryCode, 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,
@@ -3160,16 +3156,22 @@ SearchService.prototype = {
       let count = stream.available();
       let array = new Uint8Array(count);
       bis.readArrayBuffer(count, array.buffer);
 
       let bytes = Lz4.decompressFileContent(array);
       let json = JSON.parse(new TextDecoder().decode(bytes));
       if (!json.engines || !json.engines.length)
         throw "no engine in the file";
+      // Reset search default expiration on major releases
+      if (json.appVersion != Services.appinfo.version &&
+          geoSpecificDefaultsEnabled() &&
+          json.metaData) {
+        json.metaData.searchDefaultExpir = 0;
+      }
       return json;
     } catch (ex) {
       LOG("_readCacheFile: Error reading cache file: " + ex);
     } finally {
       stream.close();
     }
 
     try {
@@ -3178,17 +3180,17 @@ SearchService.prototype = {
                  createInstance(Ci.nsIFileInputStream);
       stream.init(cacheFile, MODE_RDONLY, PERMS_FILE, 0);
       let metadata = parseJsonFromStream(stream);
       let json = {};
       if ("[global]" in metadata) {
         LOG("_readCacheFile: migrating metadata from search-metadata.json");
         let data = metadata["[global]"];
         json.metaData = {};
-        let fields = ["searchDefault", "searchDefaultHash", "searchDefaultExpir",
+        let fields = ["searchDefault", "searchDefaultHash",
                       "current", "hash",
                       "visibleDefaultEngines", "visibleDefaultEnginesHash"];
         for (let field of fields) {
           let name = field.toLowerCase();
           if (name in data)
             json.metaData[field] = data[name];
         }
       }
@@ -3214,43 +3216,52 @@ SearchService.prototype = {
   async _asyncReadCacheFile() {
     let json;
     try {
       let cacheFilePath = OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME);
       let bytes = await OS.File.read(cacheFilePath, {compression: "lz4"});
       json = JSON.parse(new TextDecoder().decode(bytes));
       if (!json.engines || !json.engines.length)
         throw "no engine in the file";
+      // Reset search default expiration on major releases
+      if (json.appVersion != Services.appinfo.version &&
+          geoSpecificDefaultsEnabled() &&
+          json.metaData) {
+        json.metaData.searchDefaultExpir = 0;
+      }
       this._cacheFileJSON = json;
     } catch (ex) {
       LOG("_asyncReadCacheFile: Error reading cache file: " + ex);
       json = {};
 
       let oldMetadata =
         OS.Path.join(OS.Constants.Path.profileDir, "search-metadata.json");
       try {
         let bytes = await OS.File.read(oldMetadata);
         let metadata = JSON.parse(new TextDecoder().decode(bytes));
         if ("[global]" in metadata) {
           LOG("_asyncReadCacheFile: migrating metadata from search-metadata.json");
           let data = metadata["[global]"];
           json.metaData = {};
-          let fields = ["searchDefault", "searchDefaultHash", "searchDefaultExpir",
+          let fields = ["searchDefault", "searchDefaultHash",
                         "current", "hash",
                         "visibleDefaultEngines", "visibleDefaultEnginesHash"];
           for (let field of fields) {
             let name = field.toLowerCase();
             if (name in data)
               json.metaData[field] = data[name];
           }
         }
         delete metadata["[global]"];
         json._oldMetadata = metadata;
       } catch (ex) {}
     }
+    if (!gInitialized && json.metaData)
+      this._metaData = json.metaData;
+
     return json;
   },
 
   _batchTask: null,
   get batchTask() {
     if (!this._batchTask) {
       let task = () => {
         LOG("batchTask: Invalidating engine cache");
--- a/toolkit/components/search/tests/xpcshell/test_async_migration.js
+++ b/toolkit/components/search/tests/xpcshell/test_async_migration.js
@@ -17,10 +17,10 @@ add_task(async function test_async_metad
   await promiseAfterCache();
 
   // Check that the entries are placed as specified correctly
   let metadata = await promiseEngineMetadata();
   do_check_eq(metadata.engine.order, 1);
   do_check_eq(metadata.engine.alias, "foo");
 
   metadata = await promiseGlobalMetadata();
-  do_check_eq(metadata.searchDefaultExpir, 1471013469846);
+  do_check_false(metadata.searchDefaultExpir);
 });
--- a/toolkit/components/search/tests/xpcshell/test_geodefaults.js
+++ b/toolkit/components/search/tests/xpcshell/test_geodefaults.js
@@ -123,16 +123,63 @@ add_task(async function should_recheck_i
 
   // Check that the expiration timestamp has been updated.
   let metadata = await promiseGlobalMetadata();
   do_check_eq(typeof metadata.searchDefaultExpir, "number");
   do_check_true(metadata.searchDefaultExpir >= date + kYearInSeconds * 1000);
   do_check_true(metadata.searchDefaultExpir < date + (kYearInSeconds + 3600) * 1000);
 });
 
+add_task(async function should_recheck_if_appversion_changed() {
+  let data = await promiseCacheData();
+
+  do_check_eq(data.appVersion, Services.appinfo.version);
+
+  // Set app version to something old
+  data.appVersion = "1";
+  await promiseSaveCacheData(data);
+
+  await asyncReInit();
+  checkRequest();
+  await promiseAfterCache();
+
+  // Check that the appVersion has been updated
+  data = await promiseCacheData();
+  do_check_eq(data.appVersion, Services.appinfo.version);
+});
+
+add_task(async function should_recheck_if_appversion_changed_sync() {
+  let data = await promiseCacheData();
+
+  do_check_eq(data.appVersion, Services.appinfo.version);
+
+  // Set app version to something old
+  data.appVersion = "1";
+  await promiseSaveCacheData(data);
+
+  let commitPromise = promiseAfterCache();
+  let unInitPromise = waitForSearchNotification("uninit-complete");
+  let reInitPromise = asyncReInit();
+  await unInitPromise;
+
+  // Synchronously check the current default engine, to force a sync init.
+  // The hash is wrong, so we should fallback to the default engine from prefs.
+  do_check_false(Services.search.isInitialized);
+  Services.search.getEngines();
+  do_check_true(Services.search.isInitialized);
+
+  await reInitPromise;
+  checkRequest();
+  await commitPromise;
+
+  // Check that the appVersion has been updated
+  data = await promiseCacheData();
+  do_check_eq(data.appVersion, Services.appinfo.version);
+});
+
 add_task(async function should_recheck_when_broken_hash() {
   // This test verifies both that we ignore saved geo-defaults if the
   // hash is invalid, and that we keep the local preferences-based
   // default for all of the session in case a synchronous
   // initialization was triggered before our HTTP request completed.
 
   let metadata = await promiseGlobalMetadata();
 
--- a/toolkit/components/search/tests/xpcshell/test_sync_migration.js
+++ b/toolkit/components/search/tests/xpcshell/test_sync_migration.js
@@ -19,10 +19,10 @@ add_task(async function test_sync_metada
   await promiseAfterCache();
 
   // Check that the entries are placed as specified correctly
   let metadata = await promiseEngineMetadata();
   do_check_eq(metadata.engine.order, 1);
   do_check_eq(metadata.engine.alias, "foo");
 
   metadata = await promiseGlobalMetadata();
-  do_check_eq(metadata.searchDefaultExpir, 1471013469846);
+  do_check_false(metadata.searchDefaultExpir);
 });