Bug 1255605 - search.json.mozlz4 should always contain search engines. r=adw

toolkit/components/search/nsSearchService.js

@@ -2867,6 +2867,9 @@ SearchService.prototype = {
     }
 
     try {
+      if (!cache.engines.length)
+        throw "cannot write without any engine.";
+
       LOG("_buildCache: Writing to cache file.");
       let path = OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME);
       let data = gEncoder.encode(JSON.stringify(cache));
@@ -3185,7 +3188,10 @@ SearchService.prototype = {
       bis.readArrayBuffer(count, array.buffer);
 
       let bytes = Lz4.decompressFileContent(array);
-      return JSON.parse(new TextDecoder().decode(bytes));
+      let json = JSON.parse(new TextDecoder().decode(bytes));
+      if (!json.engines || !json.engines.length)
+        throw "no engine in the file";
+      return json;
     } catch(ex) {
       LOG("_readCacheFile: Error reading cache file: " + ex);
     } finally {
@@ -3238,6 +3244,8 @@ SearchService.prototype = {
         let cacheFilePath = OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME);
         let bytes = yield 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";
         this._cacheFileJSON = json;
       } catch (ex) {
         LOG("_asyncReadCacheFile: Error reading cache file: " + ex);

toolkit/components/search/tests/xpcshell/head_search.js

@@ -215,6 +215,12 @@ function promiseCacheData() {
   }));
 }
 
+function promiseSaveCacheData(data) {
+  return OS.File.writeAtomic(OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME),
+                             new TextEncoder().encode(JSON.stringify(data)),
+                             {compression: "lz4"});
+}
+
 function promiseEngineMetadata() {
   return new Promise(resolve => Task.spawn(function* () {
     let cache = yield promiseCacheData();
@@ -237,9 +243,7 @@ function promiseSaveGlobalMetadata(globalData) {
   return new Promise(resolve => Task.spawn(function* () {
     let data = yield promiseCacheData();
     data.metaData = globalData;
-    yield OS.File.writeAtomic(OS.Path.join(OS.Constants.Path.profileDir, CACHE_FILENAME),
-                              new TextEncoder().encode(JSON.stringify(data)),
-                              {compression: "lz4"});
+    yield promiseSaveCacheData(data);
     resolve();
   }));
 }
@@ -254,6 +258,7 @@ var forceExpiration = Task.async(function* () {
 
 /**
  * Clean the profile of any cache file left from a previous run.
+ * Returns a boolean indicating if the cache file existed.
  */
 function removeCacheFile()
 {
@@ -261,7 +266,9 @@ function removeCacheFile()
   file.append(CACHE_FILENAME);
   if (file.exists()) {
     file.remove(false);
+    return true;
   }
+  return false;
 }
 
 /**

toolkit/components/search/tests/xpcshell/test_hidden.js

@@ -45,9 +45,12 @@ add_task(function* async_init() {
 });
 
 add_task(function* sync_init() {
+  let unInitPromise = waitForSearchNotification("uninit-complete");
   let reInitPromise = asyncReInit();
-  // Synchronously check the current default engine, to force a sync init.
+  yield unInitPromise;
   do_check_false(Services.search.isInitialized);
+
+  // Synchronously check the current default engine, to force a sync init.
   do_check_eq(Services.search.currentEngine.name, "hidden");
   do_check_true(Services.search.isInitialized);
 
@@ -63,7 +66,6 @@ add_task(function* sync_init() {
   do_check_neq(engine, null);
 
   yield reInitPromise;
-  yield promiseAfterCache();
 });
 
 add_task(function* invalid_engine() {

toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js

@@ -0,0 +1,74 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function run_test() {
+  removeMetadata();
+  removeCacheFile();
+
+  do_load_manifest("data/chrome.manifest");
+
+  configureToLoadJarEngines();
+  do_check_false(Services.search.isInitialized);
+
+  run_next_test();
+}
+
+add_task(function* ignore_cache_files_without_engines() {
+  let commitPromise = promiseAfterCache()
+  yield asyncInit();
+
+  let engineCount = Services.search.getEngines().length;
+  do_check_eq(engineCount, 1);
+
+  // Wait for the file to be saved to disk, so that we can mess with it.
+  yield commitPromise;
+
+  // Remove all engines from the cache file.
+  let cache = yield promiseCacheData();
+  cache.engines = [];
+  yield promiseSaveCacheData(cache);
+
+  // Check that after an async re-initialization, we still have the same engine count.
+  commitPromise = promiseAfterCache()
+  yield asyncReInit();
+  do_check_eq(engineCount, Services.search.getEngines().length);
+  yield commitPromise;
+
+  // Check that after a sync re-initialization, we still have the same engine count.
+  yield promiseSaveCacheData(cache);
+  let unInitPromise = waitForSearchNotification("uninit-complete");
+  let reInitPromise = asyncReInit();
+  yield unInitPromise;
+  do_check_false(Services.search.isInitialized);
+  // Synchronously check the engine count; will force a sync init.
+  do_check_eq(engineCount, Services.search.getEngines().length);
+  do_check_true(Services.search.isInitialized);
+  yield reInitPromise;
+});
+
+add_task(function* skip_writing_cache_without_engines() {
+  let unInitPromise = waitForSearchNotification("uninit-complete");
+  let reInitPromise = asyncReInit();
+  yield unInitPromise;
+
+  // Configure so that no engines will be found.
+  do_check_true(removeCacheFile());
+  let resProt = Services.io.getProtocolHandler("resource")
+                        .QueryInterface(Ci.nsIResProtocolHandler);
+  resProt.setSubstitution("search-plugins",
+                          Services.io.newURI("about:blank", null, null));
+
+  // Let the async-reInit happen.
+  yield reInitPromise;
+  do_check_eq(0, Services.search.getEngines().length);
+
+  // Trigger yet another re-init, to flush of any pending cache writing task.
+  unInitPromise = waitForSearchNotification("uninit-complete");
+  reInitPromise = asyncReInit();
+  yield unInitPromise;
+
+  // Now check that a cache file doesn't exist.
+  do_check_false(removeCacheFile());
+
+  yield reInitPromise;
+});

toolkit/components/search/tests/xpcshell/xpcshell.ini

@@ -88,4 +88,5 @@ tags = addons
 [test_geodefaults.js]
 [test_hidden.js]
 [test_currentEngine_fallback.js]
+[test_require_engines_in_cache.js]
 [test_svg_icon.js]