Bug 1203167 - Remove legacy .xml file from the profile when a user-installed engine is removed, r=adw.

toolkit/components/search/nsSearchService.js

@@ -1280,7 +1280,13 @@ function Engine(aLocation, aIsReadOnly) {
   if (typeof aLocation == "string") {
     this._shortName = aLocation;
   } else if (aLocation instanceof Ci.nsILocalFile) {
-    // we already have a file (e.g. loading engines from disk)
+    if (!aIsReadOnly) {
+      // This is an engine that was installed in NS_APP_USER_SEARCH_DIR by a
+      // previous version. We are converting the file to an engine stored only
+      // in JSON, but we need to keep the reference to the profile file to
+      // remove it if the user ever removes the engine.
+      this._filePath = aLocation.persistentDescriptor;
+    }
     file = aLocation;
   } else if (aLocation instanceof Ci.nsIURI) {
     switch (aLocation.scheme) {
@@ -2061,21 +2067,18 @@ Engine.prototype = {
     this._shortName = aJson._shortName;
     this._loadPath = aJson._loadPath;
     this._description = aJson.description;
-    if (aJson._hasPreferredIcon == undefined)
-      this._hasPreferredIcon = true;
-    else
-      this._hasPreferredIcon = false;
+    this._hasPreferredIcon = aJson._hasPreferredIcon == undefined;
     this._queryCharset = aJson.queryCharset || DEFAULT_QUERY_CHARSET;
     this.__searchForm = aJson.__searchForm;
     this._updateInterval = aJson._updateInterval || null;
     this._updateURL = aJson._updateURL || null;
     this._iconUpdateURL = aJson._iconUpdateURL || null;
-    if (aJson._readOnly == undefined)
-      this._readOnly = true;
-    else
-      this._readOnly = false;
+    this._readOnly = aJson._readOnly == undefined;
     this._iconURI = makeURI(aJson._iconURL);
     this._iconMapObj = aJson._iconMapObj;
+    if (aJson.filePath) {
+      this._filePath = aJson.filePath;
+    }
     if (aJson.extensionID) {
       this._extensionID = aJson.extensionID;
     }
@@ -2118,6 +2121,9 @@ Engine.prototype = {
       json.queryCharset = this.queryCharset;
     if (!this._readOnly)
       json._readOnly = this._readOnly;
+    if (this._filePath) {
+      json.filePath = this._filePath;
+    }
     if (this._extensionID) {
       json.extensionID = this._extensionID;
     }
@@ -2125,20 +2131,6 @@ Engine.prototype = {
     return json;
   },
 
-  /**
-   * Remove the engine's file from disk. The search service calls this once it
-   * removes the engine from its internal store. This function will throw if
-   * the file cannot be removed.
-   */
-  _remove: function SRCH_ENG_remove() {
-    if (this._readOnly)
-      FAIL("Can't remove read only engine!", Cr.NS_ERROR_FAILURE);
-    if (!this._file || !this._file.exists())
-      FAIL("Can't remove engine: file doesn't exist!", Cr.NS_ERROR_FILE_NOT_FOUND);
-
-    this._file.remove(false);
-  },
-
   // nsISearchEngine
   get alias() {
     if (this._alias === undefined)
@@ -3878,12 +3870,15 @@ SearchService.prototype = {
       engineToRemove.hidden = true;
       engineToRemove.alias = null;
     } else {
-      // Remove the engine file from disk (this might throw)
-      //FIXME _file won't be set for converted engines, so this code is almost dead.
-      if (engineToRemove._file &&
-          engineToRemove._file.exists())
-        engineToRemove._remove();
-      engineToRemove._file = null;
+      // Remove the engine file from disk if we had a legacy file in the profile.
+      if (engineToRemove._filePath) {
+        let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsILocalFile);
+        file.persistentDescriptor = engineToRemove._filePath;
+        if (file.exists()) {
+          file.remove(false);
+        }
+        engineToRemove._filePath = null;
+      }
 
       // Remove the engine from _sortedEngines
       var index = this._sortedEngines.indexOf(engineToRemove);

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

@@ -0,0 +1,35 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const NS_APP_USER_SEARCH_DIR  = "UsrSrchPlugns";
+
+function run_test() {
+  do_test_pending();
+
+  // Copy an engine to [profile]/searchplugin/
+  let dir = Services.dirsvc.get(NS_APP_USER_SEARCH_DIR, Ci.nsIFile);
+  if (!dir.exists())
+    dir.create(dir.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+  do_get_file("data/engine-override.xml").copyTo(dir, "bug645970.xml");
+
+  let file = dir.clone();
+  file.append("bug645970.xml");
+  do_check_true(file.exists());
+
+  do_check_false(Services.search.isInitialized);
+
+  Services.search.init(function search_initialized(aStatus) {
+    do_check_true(Components.isSuccessCode(aStatus));
+    do_check_true(Services.search.isInitialized);
+
+    // test the engine is loaded ok.
+    let engine = Services.search.getEngineByName("bug645970");
+    do_check_neq(engine, null);
+
+    // remove the engine and verify the file has been removed too.
+    Services.search.removeEngine(engine);
+    do_check_false(file.exists());
+
+    do_test_finished();
+  });
+}

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

@@ -76,6 +76,7 @@ tags = addons
 [test_sync_delay_fallback.js]
 [test_sync_profile_engine.js]
 [test_rel_searchform.js]
+[test_remove_profile_engine.js]
 [test_selectedEngine.js]
 [test_geodefaults.js]
 [test_hidden.js]