Bug 841554 - Part 1: extend nsISearchEngine to include identifier. r=gavin, a=bajaj
authorRichard Newman <rnewman@mozilla.com>
Mon, 25 Mar 2013 10:53:40 -0400
changeset 132495 fc077ab0cebf06003fd9bf75c8fc8d32e5e50e26
parent 132494 90b3c33b1eddc019ea175940ac908cdf88111cf6
child 132496 f6d8dfa7859dba2523952a4ece8d0a9c23c3e0e2
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin, bajaj
bugs841554
milestone21.0a2
Bug 841554 - Part 1: extend nsISearchEngine to include identifier. r=gavin, a=bajaj
netwerk/base/public/nsIBrowserSearchService.idl
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_identifiers.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/netwerk/base/public/nsIBrowserSearchService.idl
+++ b/netwerk/base/public/nsIBrowserSearchService.idl
@@ -17,17 +17,17 @@ interface nsISearchSubmission : nsISuppo
   readonly attribute nsIInputStream postData;
 
   /**
    * The URI to submit a search to.
    */
   readonly attribute nsIURI uri;
 };
 
-[scriptable, uuid(6839f025-2e25-408e-892e-c2c2fa5650c5)]
+[scriptable, uuid(ccf6aa20-10a9-4a0c-a81d-31b10ea846de)]
 interface nsISearchEngine : nsISupports
 {
   /**
    * Gets a nsISearchSubmission object that contains information about what to
    * send to the search engine, including the URI and postData, if applicable.
    *
    * @param  data
    *         Data to add to the submission object.
@@ -127,16 +127,22 @@ interface nsISearchEngine : nsISupports
    */
   readonly attribute AString searchForm;
 
   /**
    * The search engine type.
    */
   readonly attribute long type;
 
+  /**
+   * An optional unique identifier for this search engine within the context of
+   * the distribution, as provided by the distributing entity.
+   */
+  readonly attribute AString identifier;
+
 };
 
 /**
  * Callback for asynchronous initialization of nsIBrowserSearchService
  */
 [scriptable, function, uuid(02256156-16e4-47f1-9979-76ff98ceb590)]
 interface nsIBrowserSearchInitObserver : nsISupports
 {
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -1081,16 +1081,19 @@ function Engine(aLocation, aSourceDataTy
     ERROR("Engine location is neither a File nor a URI object",
           Cr.NS_ERROR_INVALID_ARG);
 }
 
 Engine.prototype = {
   // The engine's alias (can be null). Initialized to |undefined| to indicate
   // not-initialized-from-engineMetadataService.
   _alias: undefined,
+  // A distribution-unique identifier for the engine. Either null or set
+  // when loaded. See getter.
+  _identifier: undefined,
   // The data describing the engine. Is either an array of bytes, for Sherlock
   // files, or an XML document element, for XML plugins.
   _data: null,
   // The engine's data type. See data types (DATA_) defined above.
   _dataType: null,
   // Whether or not the engine is readonly.
   _readOnly: true,
   // The engine's description
@@ -2275,16 +2278,48 @@ Engine.prototype = {
     return this._alias;
   },
   set alias(val) {
     this._alias = val;
     engineMetadataService.setAttr(this, "alias", val);
     notifyAction(this, SEARCH_ENGINE_CHANGED);
   },
 
+  /**
+   * Return the built-in identifier of app-provided engines.
+   *
+   * Note that this identifier is substantially similar to _id, with the
+   * following exceptions:
+   *
+   * * There is no trailing file extension.
+   * * There is no [app] prefix.
+   *
+   * @return a string identifier, or null.
+   */
+  get identifier() {
+    if (this._identifier !== undefined) {
+      return this._identifier;
+    }
+
+    // No identifier if If the engine isn't app-provided
+    if (!this._isInAppDir && !this._isInJAR) {
+      return this._identifier = null;
+    }
+
+    let leaf = this._getLeafName();
+    ENSURE_WARN(leaf, "identifier: app-provided engine has no leafName");
+
+    // Strip file extension.
+    let ext = leaf.lastIndexOf(".");
+    if (ext == -1) {
+      return this._identifier = leaf;
+    }
+    return this._identifier = leaf.substring(0, ext);
+  },
+
   get description() {
     return this._description;
   },
 
   get hidden() {
     if (this._hidden === null)
       this._hidden = engineMetadataService.getAttr(this, "hidden") || false;
     return this._hidden;
@@ -2318,55 +2353,69 @@ Engine.prototype = {
       return this._file.path;
 
     if (this._uri)
       return this._uri.spec;
 
     return "";
   },
 
+  /**
+   * @return the leaf name of the filename or URI of this plugin,
+   *         or null if no file or URI is known.
+   */
+  _getLeafName: function () {
+    if (this._file) {
+      return this._file.leafName;
+    }
+    if (this._uri && this._uri instanceof Ci.nsIURL) {
+      return this._uri.fileName;
+    }
+    return null;
+  },
+    
   // The file that the plugin is loaded from is a unique identifier for it.  We
   // use this as the identifier to store data in the sqlite database
   __id: null,
   get _id() {
-    if (this.__id)
+    if (this.__id) {
       return this.__id;
+    }
+
+    let leafName = this._getLeafName();
 
     // Treat engines loaded from JARs the same way we treat app shipped
     // engines.
     // Theoretically, these could also come from extensions, but there's no
     // real way for extensions to register their chrome locations at the
     // moment, so let's not deal with that case.
     // This means we're vulnerable to conflicts if a file loaded from a JAR
     // has the same filename as a file loaded from the app dir, but with a
     // different engine name. People using the JAR functionality should be
     // careful not to do that!
     if (this._isInAppDir || this._isInJAR) {
-      let leafName;
-      if (this._file)
-        leafName = this._file.leafName;
-      else {
-        // If we've reached this point, we must be loaded from a JAR, which
-        // also means we should have a URL.
-        ENSURE_WARN(this._isInJAR && (this._uri instanceof Ci.nsIURL),
-                    "_id: not inJAR, or no URI", Cr.NS_ERROR_UNEXPECTED);
-        leafName = this._uri.fileName;
-      }
-
+      // App dir and JAR engines should always have leafNames
+      ENSURE_WARN(leafName, "_id: no leafName for appDir or JAR engine",
+                  Cr.NS_ERROR_UNEXPECTED);
       return this.__id = "[app]/" + leafName;
     }
 
-    ENSURE_WARN(this._file, "_id: no _file!", Cr.NS_ERROR_UNEXPECTED);
-
-    if (this._isInProfile)
-      return this.__id = "[profile]/" + this._file.leafName;
+    if (this._isInProfile) {
+      ENSURE_WARN(leafName, "_id: no leafName for profile engine",
+                  Cr.NS_ERROR_UNEXPECTED);
+      return this.__id = "[profile]/" + leafName;
+    }
+
+    // If the engine isn't a JAR engine, it should have a file.
+    ENSURE_WARN(this._file, "_id: no _file for non-JAR engine",
+                Cr.NS_ERROR_UNEXPECTED);
 
     // We're not in the profile or appdir, so this must be an extension-shipped
     // plugin. Use the full filename.
-    return this.__id  = this._file.path;
+    return this.__id = this._file.path;
   },
 
   get _installLocation() {
     if (this.__installLocation === null) {
       if (!this._file) {
         ENSURE_WARN(this._uri, "Engines without files must have URIs",
                     Cr.NS_ERROR_UNEXPECTED);
         this.__installLocation = SEARCH_JAR;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/search/tests/xpcshell/test_identifiers.js
@@ -0,0 +1,60 @@
+/* Any copyright is dedicated to the Public Domain.
+ *    http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Test that a search engine's identifier can be extracted from the filename.
+ */
+
+"use strict";
+
+const Ci = Components.interfaces;
+const SEARCH_APP_DIR = 1;
+
+function run_test() {
+  removeMetadata();
+  removeCacheFile();
+  do_load_manifest("data/chrome.manifest");
+
+  let url  = "chrome://testsearchplugin/locale/searchplugins/";
+  Services.prefs.setCharPref("browser.search.jarURIs", url);
+  Services.prefs.setBoolPref("browser.search.loadFromJars", true);
+
+  updateAppInfo();
+
+  run_next_test();
+}
+
+add_test(function test_identifier() {
+  let engineFile = gProfD.clone();
+  engineFile.append("searchplugins");
+  engineFile.append("test-search-engine.xml");
+  engineFile.parent.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+
+  // Copy the test engine to the test profile.
+  let engineTemplateFile = do_get_file("data/engine.xml");
+  engineTemplateFile.copyTo(engineFile.parent, "test-search-engine.xml");
+
+  let search = Services.search.init(function initComplete(aResult) {
+    do_print("init'd search service");
+    do_check_true(Components.isSuccessCode(aResult));
+
+    let profileEngine = Services.search.getEngineByName("Test search engine");
+    let jarEngine = Services.search.getEngineByName("bug645970");
+
+    do_check_true(profileEngine instanceof Ci.nsISearchEngine);
+    do_check_true(jarEngine instanceof Ci.nsISearchEngine);
+
+    // An engine loaded from the profile directory won't have an identifier,
+    // because it's not built-in.
+    do_check_eq(profileEngine.identifier, null);
+
+    // An engine loaded from a JAR will have an identifier corresponding to
+    // the filename inside the JAR. (In this case it's the same as the name.)
+    do_check_eq(jarEngine.identifier, "bug645970");
+
+    removeMetadata();
+    removeCacheFile();
+    run_next_test();
+  });
+});
+
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 head = head_search.js
 tail = 
 firefox-appdir = browser
 
 [test_645970.js]
+[test_identifiers.js]
 [test_init_async_multiple.js]
 [test_init_async_multiple_then_sync.js]
 [test_json_cache.js]
 [test_migratedb.js]
 [test_nodb.js]
 [test_nodb_pluschanges.js]
 [test_purpose.js]