Bug 725943 - Refactor nsSearchService to generalize use of Lazy. r=MattN

toolkit/components/search/nsSearchService.js

@@ -1191,8 +1191,8 @@ Engine.prototype = {
   _updateURL: null,
   // The url to check for a new icon
   _iconUpdateURL: null,
-  // A reference to the timer used for lazily serializing the engine to file
-  _serializeTimer: null,
+  /* Deferred serialization task. */
+  _lazySerializeTask: null,
 
   /**
    * Retrieves the data from the engine's file. If the engine's dataType is
@@ -2363,28 +2363,15 @@ Engine.prototype = {
     return doc;
   },
 
-  _lazySerializeToFile: function SRCH_ENG_serializeToFile() {
-    if (this._serializeTimer) {
-      // Reset the timer
-      this._serializeTimer.delay = LAZY_SERIALIZE_DELAY;
-    } else {
-      this._serializeTimer = Cc["@mozilla.org/timer;1"].
-                             createInstance(Ci.nsITimer);
-      var timerCallback = {
-        self: this,
-        notify: function SRCH_ENG_notify(aTimer) {
-          try {
-            this.self._serializeToFile();
-          } catch (ex) {
-            LOG("Serialization from timer callback failed:\n" + ex);
-          }
-          this.self._serializeTimer = null;
-        }
-      };
-      this._serializeTimer.initWithCallback(timerCallback,
-                                            LAZY_SERIALIZE_DELAY,
-                                            Ci.nsITimer.TYPE_ONE_SHOT);
+  get lazySerializeTask() {
+    if (!this._lazySerializeTask) {
+      let task = function taskCallback() {
+        this._serializeToFile();
+      }.bind(this);
+      this._lazySerializeTask = new DeferredTask(task, LAZY_SERIALIZE_DELAY);
     }
+
+    return this._lazySerializeTask;
   },
 
   /**
@@ -2415,6 +2402,9 @@ Engine.prototype = {
     }
 
     closeSafeOutputStream(fos);
+
+    Services.obs.notifyObservers(file.clone(), SEARCH_SERVICE_TOPIC,
+                                 "write-engine-to-disk-complete");
   },
 
   /**
@@ -2660,7 +2650,7 @@ Engine.prototype = {
     url.addParam(aName, aValue);
 
     // Serialize the changes to file lazily
-    this._lazySerializeToFile();
+    this.lazySerializeTask.start();
   },
 
 #ifdef ANDROID
@@ -3236,25 +3226,16 @@ SearchService.prototype = {
     });
   },
 
-  _batchTimer: null,
-  _batchCacheInvalidation: function SRCH_SVC__batchCacheInvalidation() {
-    let callback = {
-      self: this,
-      notify: function SRCH_SVC_batchTimerNotify(aTimer) {
-        LOG("_batchCacheInvalidation: Invalidating engine cache");
-        this.self._buildCache();
-        this.self._batchTimer = null;
-      }
-    };
-
-    if (!this._batchTimer) {
-      this._batchTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-      this._batchTimer.initWithCallback(callback, CACHE_INVALIDATION_DELAY,
-                                        Ci.nsITimer.TYPE_ONE_SHOT);
-    } else {
-      this._batchTimer.delay = CACHE_INVALIDATION_DELAY;
-      LOG("_batchCacheInvalidation: Batch timer reset");
+  _batchTask: null,
+  get batchTask() {
+    if (!this._batchTask) {
+      let task = function taskCallback() {
+        LOG("batchTask: Invalidating engine cache");
+        this._buildCache();
+      }.bind(this);
+      this._batchTask = new DeferredTask(task, CACHE_INVALIDATION_DELAY);
     }
+    return this._batchTask;
   },
 
   _addEngineToStore: function SRCH_SVC_addEngineToStore(aEngine) {
@@ -3885,7 +3866,7 @@ SearchService.prototype = {
     engine._initFromMetadata(aName, aIconURL, aAlias, aDescription,
                              aMethod, aTemplate);
     this._addEngineToStore(engine);
-    this._batchCacheInvalidation();
+    this.batchTask.start();
   },
 
   addEngine: function SRCH_SVC_addEngine(aEngineURL, aDataType, aIconURL,
@@ -3948,10 +3929,10 @@ SearchService.prototype = {
       engineToRemove.hidden = true;
       engineToRemove.alias = null;
     } else {
-      // Cancel the lazy serialization timer if it's running
-      if (engineToRemove._serializeTimer) {
-        engineToRemove._serializeTimer.cancel();
-        engineToRemove._serializeTimer = null;
+      // Cancel the serialized task if it's running
+      if (engineToRemove._lazySerializeTask) {
+        engineToRemove._lazySerializeTask.cancel();
+        engineToRemove._lazySerializeTask = null;
       }
 
       // Remove the engine file from disk (this might throw)
@@ -4149,21 +4130,20 @@ SearchService.prototype = {
               LOG("nsSearchService::observe: setting current");
               this.currentEngine = aEngine;
             }
-            this._batchCacheInvalidation();
+            this.batchTask.start();
             break;
           case SEARCH_ENGINE_CHANGED:
           case SEARCH_ENGINE_REMOVED:
-            this._batchCacheInvalidation();
+            this.batchTask.start();
             break;
         }
         break;
 
       case QUIT_APPLICATION_TOPIC:
         this._removeObservers();
-        if (this._batchTimer) {
+        if (this._batchTask) {
           // Flush to disk immediately
-          this._batchTimer.cancel();
-          this._buildCache();
+          this._batchTask.flush();
         }
         engineMetadataService.flush();
         break;

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

@@ -0,0 +1,92 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * test_serialize_file: Add test engines
+ *
+ * Ensure that :
+ * - File is created.
+ * - File size is bigger than 0.
+ * - lazySerializeTask updates the file.
+ */
+
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+
+Cu.import("resource://testing-common/httpd.js");
+Cu.import("resource://gre/modules/NetUtil.jsm");
+
+add_test(function test_batchTask() {
+  let observer = function(aSubject, aTopic, aData) {
+    if (aTopic == "browser-search-engine-modified" && aData == "engine-loaded") {
+      let engine1 = Services.search.getEngineByName("Test search engine");
+      let engine2 = Services.search.getEngineByName("Sherlock test search engine");
+      if (engine1 && engine2) {
+        Services.obs.removeObserver(observer, aTopic);
+        // Test that files are written correctly.
+        let engineFile1 = engine1.wrappedJSObject._file;
+        let engineFile2 = engine2.wrappedJSObject._file;
+        do_check_true(engineFile1.exists());
+        do_check_true(engineFile2.exists());
+        do_check_neq(engineFile1.fileSize, 0);
+        do_check_neq(engineFile2.fileSize, 0);
+        run_next_test();
+      }
+    }
+  }
+
+  Services.obs.addObserver(observer, "browser-search-engine-modified", false);
+  Services.search.addEngine("http://localhost:4444/data/engine.xml",
+                            Ci.nsISearchEngine.DATA_XML, null, false);
+  Services.search.addEngine("http://localhost:4444/data/engine.src",
+                            Ci.nsISearchEngine.DATA_TEXT,
+                            "http://localhost:4444/data/ico-size-16x16-png.ico",
+                            false);
+});
+
+add_test(function test_addParam() {
+  let engine = Services.search.getEngineByName("Test search engine");
+  engine.addParam("param-name", "param-value", null);
+
+  function readAsyncFile(aFile, aCallback) {
+    NetUtil.asyncFetch(aFile, function(inputStream, status) {
+      do_check_true(Components.isSuccessCode(status));
+
+      let data = NetUtil.readInputStreamToString(inputStream, inputStream.available());
+      aCallback(data);
+    });
+  }
+
+  let observer = function(aSubject, aTopic, aData) {
+    // The sherlock engine file may still be updated because the icon takes
+    // time be loaded, therefore, the engine name is checked here.
+    aSubject.QueryInterface(Ci.nsIFile);
+    if (aTopic == "browser-search-service" &&
+        aData == "write-engine-to-disk-complete" &&
+        aSubject.leafName == "test-search-engine.xml") {
+      Services.obs.removeObserver(observer, aTopic);
+
+      let engineFile = engine.wrappedJSObject._file;
+
+      readAsyncFile(engineFile, function(engineData) {
+        do_check_true(engineData.indexOf("param-name") > 0);
+        run_next_test();
+      });
+    }
+  }
+  Services.obs.addObserver(observer, "browser-search-service", false);
+});
+
+function run_test() {
+  updateAppInfo();
+
+  let httpServer = new HttpServer();
+  httpServer.start(4444);
+  httpServer.registerDirectory("/", do_get_cwd());
+
+  do_register_cleanup(function cleanup() {
+    httpServer.stop(function() {});
+  });
+
+  run_next_test();
+}

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

@@ -29,6 +29,7 @@ support-files =
 [test_notifications.js]
 [test_addEngine_callback.js]
 [test_multipleIcons.js]
+[test_serialize_file.js]
 [test_async.js]
 [test_sync.js]
 [test_sync_fallback.js]