Bug 1457321: Part 3 - Load built-in dictionaries from omnijar rather than scanning directories. r=aswan,ehsan
authorKris Maglione <maglione.k@gmail.com>
Fri, 27 Apr 2018 13:43:50 -0700
changeset 420056 8810007550b1
parent 420055 d31a5556d42a
child 420057 43a358272d5e
push id34060
push usercsabou@mozilla.com
push dateSun, 27 May 2018 13:06:48 +0000
treeherdermozilla-central@6b9076ac236c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, ehsan
bugs1457321
milestone62.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 1457321: Part 3 - Load built-in dictionaries from omnijar rather than scanning directories. r=aswan,ehsan MozReview-Commit-ID: CgYHrcJsYfE
extensions/spellcheck/hunspell/glue/mozHunspell.cpp
extensions/spellcheck/idl/mozISpellCheckingEngine.idl
toolkit/components/extensions/Extension.jsm
toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_dictionary.js
toolkit/mozapps/extensions/test/xpcshell/test_dictionary_webextension.js
--- a/extensions/spellcheck/hunspell/glue/mozHunspell.cpp
+++ b/extensions/spellcheck/hunspell/glue/mozHunspell.cpp
@@ -59,23 +59,20 @@
 
 #include "mozHunspell.h"
 #include "nsReadableUtils.h"
 #include "nsString.h"
 #include "nsIObserverService.h"
 #include "nsISimpleEnumerator.h"
 #include "nsIDirectoryEnumerator.h"
 #include "nsIFile.h"
-#include "nsDirectoryServiceUtils.h"
-#include "nsDirectoryServiceDefs.h"
 #include "mozISpellI18NManager.h"
 #include "nsUnicharUtils.h"
 #include "nsCRT.h"
 #include "mozInlineSpellChecker.h"
-#include "mozilla/Services.h"
 #include <stdlib.h>
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsNetUtil.h"
 #include "mozilla/dom/ContentParent.h"
 
 using mozilla::dom::ContentParent;
 using namespace mozilla;
@@ -293,61 +290,31 @@ NS_IMETHODIMP mozHunspell::GetDictionary
 
 void
 mozHunspell::LoadDictionaryList(bool aNotifyChildProcesses)
 {
   mDictionaries.Clear();
 
   nsresult rv;
 
-  nsCOMPtr<nsIProperties> dirSvc =
-    do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID);
-  if (!dirSvc)
-    return;
-
   // find built in dictionaries, or dictionaries specified in
   // spellchecker.dictionary_path in prefs
   nsCOMPtr<nsIFile> dictDir;
 
   // check preferences first
   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
   if (prefs) {
     nsAutoCString extDictPath;
     rv = prefs->GetCharPref("spellchecker.dictionary_path", extDictPath);
     if (NS_SUCCEEDED(rv)) {
       // set the spellchecker.dictionary_path
       rv = NS_NewNativeLocalFile(extDictPath, true, getter_AddRefs(dictDir));
     }
-  }
-  if (!dictDir) {
-    // spellcheck.dictionary_path not found, set internal path
-    rv = dirSvc->Get(DICTIONARY_SEARCH_DIRECTORY,
-                     NS_GET_IID(nsIFile), getter_AddRefs(dictDir));
-  }
-  if (dictDir) {
-    LoadDictionariesFromDir(dictDir);
-  }
-  else {
-    // try to load gredir/dictionaries
-    nsCOMPtr<nsIFile> greDir;
-    rv = dirSvc->Get(NS_GRE_DIR,
-                     NS_GET_IID(nsIFile), getter_AddRefs(greDir));
-    if (NS_SUCCEEDED(rv)) {
-      greDir->AppendNative(NS_LITERAL_CSTRING("dictionaries"));
-      LoadDictionariesFromDir(greDir);
-    }
-
-    // try to load appdir/dictionaries only if different than gredir
-    nsCOMPtr<nsIFile> appDir;
-    rv = dirSvc->Get(NS_XPCOM_CURRENT_PROCESS_DIR,
-                     NS_GET_IID(nsIFile), getter_AddRefs(appDir));
-    bool equals;
-    if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(appDir->Equals(greDir, &equals)) && !equals) {
-      appDir->AppendNative(NS_LITERAL_CSTRING("dictionaries"));
-      LoadDictionariesFromDir(appDir);
+    if (dictDir) {
+      LoadDictionariesFromDir(dictDir);
     }
   }
 
   // find dictionaries in DICPATH
   char* dicEnv = PR_GetEnv("DICPATH");
   if (dicEnv) {
     // do a two-pass dance so dictionaries are loaded right-to-left as preference
     nsTArray<nsCOMPtr<nsIFile>> dirs;
@@ -639,20 +606,22 @@ NS_IMETHODIMP mozHunspell::AddDictionary
   NS_ENSURE_TRUE(aFile, NS_ERROR_INVALID_ARG);
 
   mDynamicDictionaries.Put(aLang, aFile);
   mDictionaries.Put(aLang, aFile);
   DictionariesChanged(true);
   return NS_OK;
 }
 
-NS_IMETHODIMP mozHunspell::RemoveDictionary(const nsAString& aLang, nsIURI *aFile)
+NS_IMETHODIMP mozHunspell::RemoveDictionary(const nsAString& aLang, nsIURI *aFile, bool* aRetVal)
 {
   NS_ENSURE_TRUE(aFile, NS_ERROR_INVALID_ARG);
+  *aRetVal = false;
 
   nsCOMPtr<nsIURI> file = mDynamicDictionaries.Get(aLang);
   bool equal;
   if (file && NS_SUCCEEDED(file->Equals(aFile, &equal)) && equal) {
     mDynamicDictionaries.Remove(aLang);
     LoadDictionaryList(true);
+    *aRetVal = true;
   }
   return NS_OK;
 }
--- a/extensions/spellcheck/idl/mozISpellCheckingEngine.idl
+++ b/extensions/spellcheck/idl/mozISpellCheckingEngine.idl
@@ -94,18 +94,18 @@ interface mozISpellCheckingEngine : nsIS
    * ".dic" extension.
    */
   void addDictionary(in AString lang, in nsIURI file);
 
   /**
    * Remove a dictionary with the given language code and path. If the path does
    * not match that of the current entry with the given language code, it is not
    * removed.
+   *
+   * @returns True if the dictionary was found and removed.
    */
-  void removeDictionary(in AString lang, in nsIURI file);
+  bool removeDictionary(in AString lang, in nsIURI file);
 };
 
 %{C++
-#define DICTIONARY_SEARCH_DIRECTORY "DictD"
-
 #define SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION \
   "spellcheck-dictionary-remove"
 %}
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -49,16 +49,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
   L10nRegistry: "resource://gre/modules/L10nRegistry.jsm",
   Log: "resource://gre/modules/Log.jsm",
   MessageChannel: "resource://gre/modules/MessageChannel.jsm",
   NetUtil: "resource://gre/modules/NetUtil.jsm",
   OS: "resource://gre/modules/osfile.jsm",
   PluralForm: "resource://gre/modules/PluralForm.jsm",
   Schemas: "resource://gre/modules/Schemas.jsm",
   TelemetryStopwatch: "resource://gre/modules/TelemetryStopwatch.jsm",
+  XPIProvider: "resource://gre/modules/addons/XPIProvider.jsm",
 });
 
 XPCOMUtils.defineLazyGetter(
   this, "processScript",
   () => Cc["@mozilla.org/webextensions/extension-process-script;1"]
           .getService().wrappedJSObject);
 
 // This is used for manipulating jar entry paths, which always use Unix
@@ -1845,19 +1846,17 @@ class Dictionary extends ExtensionData {
       spellCheck.addDictionary(lang, uri);
     }
 
     Management.emit("ready", this);
   }
 
   async shutdown(reason) {
     if (reason !== "APP_SHUTDOWN") {
-      for (let [lang, file] of Object.entries(this.dictionaries)) {
-        spellCheck.removeDictionary(lang, file);
-      }
+      XPIProvider.unregisterDictionaries(this.dictionaries);
     }
   }
 }
 
 class Langpack extends ExtensionData {
   constructor(addonData, startupReason) {
     super(addonData.resourceURI);
     this.startupData = addonData.startupData;
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -801,16 +801,20 @@ var AddonTestUtils = {
     // the AddonManagerInternal.shutdown() promise
     let shutdownError = XPIscope.XPIDatabase._saveError;
 
     AddonManagerPrivate.unregisterProvider(XPIscope.XPIProvider);
     Cu.unload("resource://gre/modules/addons/XPIProvider.jsm");
     Cu.unload("resource://gre/modules/addons/XPIDatabase.jsm");
     Cu.unload("resource://gre/modules/addons/XPIInstall.jsm");
 
+    let ExtensionScope = ChromeUtils.import("resource://gre/modules/Extension.jsm", null);
+    ChromeUtils.defineModuleGetter(ExtensionScope, "XPIProvider",
+                                   "resource://gre/modules/addons/XPIProvider.jsm");
+
     if (shutdownError)
       throw shutdownError;
 
     return true;
   },
 
   /**
    * Asynchronously restart the AddonManager.  If newVersion is provided,
@@ -1562,27 +1566,36 @@ var AddonTestUtils = {
           Management.off("ready", listener);
           resolve(extension);
         }
       });
     });
   },
 
   /**
+   * Initializes the URLPreloader, which is required in order to load
+   * built_in_addons.json. This has the side-effect of setting
+   * preferences which flip Cu.isInAutomation to true.
+   */
+  initializeURLPreloader() {
+    Services.prefs.setBoolPref(PREF_DISABLE_SECURITY, true);
+    aomStartup.initializeURLPreloader();
+  },
+
+  /**
    * Override chrome URL for specifying allowed built-in add-ons.
    *
    * @param {object} data - An object specifying which add-on IDs are permitted
    *                        to load, for instance: { "system": ["id1", "..."] }
    */
   async overrideBuiltIns(data) {
     // We need to set this in order load the URL preloader service, which
     // is only possible when running in automation.
     let prevPrefVal = Services.prefs.getBoolPref(PREF_DISABLE_SECURITY, false);
-    Services.prefs.setBoolPref(PREF_DISABLE_SECURITY, true);
-    aomStartup.initializeURLPreloader();
+    this.initializeURLPreloader();
 
     let file = this.tempDir.clone();
     file.append("override.txt");
     this.tempXPIs.push(file);
 
     let manifest = Services.io.newFileURI(file);
     await OS.File.writeAtomic(file.path,
       new TextEncoder().encode(JSON.stringify(data)));
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -40,16 +40,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
 
   XPIDatabase: "resource://gre/modules/addons/XPIDatabase.jsm",
   XPIDatabaseReconcile: "resource://gre/modules/addons/XPIDatabase.jsm",
   XPIInstall: "resource://gre/modules/addons/XPIInstall.jsm",
 });
 
 XPCOMUtils.defineLazyServiceGetters(this, {
   aomStartup: ["@mozilla.org/addons/addon-manager-startup;1", "amIAddonManagerStartup"],
+  spellCheck: ["@mozilla.org/spellchecker/engine;1", "mozISpellCheckingEngine"],
   timerManager: ["@mozilla.org/updates/timer-manager;1", "nsIUpdateTimerManager"],
 });
 
 const nsIFile = Components.Constructor("@mozilla.org/file/local;1", "nsIFile",
                                        "initWithPath");
 const FileInputStream = Components.Constructor("@mozilla.org/network/file-input-stream;1",
                                                "nsIFileInputStream", "init");
 
@@ -948,25 +949,17 @@ class BuiltInLocation extends DirectoryL
    * for each.
    *
    * @returns {Map<AddonID, nsIFile>}
    *        A map of add-ons present in this location.
    */
   readAddons() {
     let addons = new Map();
 
-    let manifest;
-    try {
-      let url = Services.io.newURI(BUILT_IN_ADDONS_URI);
-      let data = Cu.readUTF8URI(url);
-      manifest = JSON.parse(data);
-    } catch (e) {
-      logger.warn("List of valid built-in add-ons could not be parsed.", e);
-      return addons;
-    }
+    let manifest = XPIProvider.builtInAddons;
 
     if (!("system" in manifest)) {
       logger.warn("No list of valid system add-ons found.");
       return addons;
     }
 
     for (let id of manifest.system) {
       let file = this.dir.clone();
@@ -2018,16 +2011,55 @@ var XPIProvider = {
         } catch (e) {
           logger.warn(`Failed to add ${constructor.name} install location ${name}`, e);
         }
       }
     }
   },
 
   /**
+   * Registers the built-in set of dictionaries with the spell check
+   * service.
+   */
+  registerBuiltinDictionaries() {
+    this.dictionaries = {};
+    for (let [lang, path] of Object.entries(this.builtInAddons.dictionaries || {})) {
+      path = path.slice(0, -4) + ".aff";
+      let uri = Services.io.newURI(`resource://gre/${path}`);
+
+      this.dictionaries[lang] = uri;
+      spellCheck.addDictionary(lang, uri);
+    }
+  },
+
+  /**
+   * Unregisters the dictionaries in the given object, and re-registers
+   * any built-in dictionaries in their place, when they exist.
+   *
+   * @param {Object<nsIURI>} aDicts
+   *        An object containing a property with a dictionary language
+   *        code and a nsIURI value for each dictionary to be
+   *        unregistered.
+   */
+  unregisterDictionaries(aDicts) {
+    let origDict = spellCheck.dictionary;
+
+    for (let [lang, uri] of Object.entries(aDicts)) {
+      if (spellCheck.removeDictionary(lang, uri) &&
+          this.dictionaries.hasOwnProperty(lang)) {
+        spellCheck.addDictionary(lang, this.dictionaries[lang]);
+
+        if (lang == origDict) {
+          spellCheck.dictionary = origDict;
+        }
+      }
+    }
+  },
+
+  /**
    * Starts the XPI provider initializes the install locations and prefs.
    *
    * @param {boolean?} aAppChanged
    *        A tri-state value. Undefined means the current profile was created
    *        for this session, true means the profile already existed but was
    *        last used with an application with a different version number,
    *        false means that the profile was last used by this version of the
    *        application.
@@ -2039,16 +2071,27 @@ var XPIProvider = {
    *        if it is a new profile or the version is unknown
    */
   startup(aAppChanged, aOldAppVersion, aOldPlatformVersion) {
     try {
       AddonManagerPrivate.recordTimestamp("XPI_startup_begin");
 
       logger.debug("startup");
 
+      this.builtInAddons = {};
+      try {
+        let url = Services.io.newURI(BUILT_IN_ADDONS_URI);
+        let data = Cu.readUTF8URI(url);
+        this.builtInAddons = JSON.parse(data);
+      } catch (e) {
+        logger.warn("List of valid built-in add-ons could not be parsed.", e);
+      }
+
+      this.registerBuiltinDictionaries();
+
       // Clear this at startup for xpcshell test restarts
       this._telemetryDetails = {};
       // Register our details structure with AddonManager
       AddonManagerPrivate.setTelemetryDetails("XPI", this._telemetryDetails);
 
       this.setupInstallLocations(aAppChanged);
 
       if (!AppConstants.MOZ_REQUIRE_SIGNING || Cu.isInAutomation)
--- a/toolkit/mozapps/extensions/test/xpcshell/test_dictionary.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_dictionary.js
@@ -2,16 +2,19 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 // This verifies that bootstrappable add-ons can be used without restarts.
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 Cu.importGlobalProperties(["XMLHttpRequest"]);
 
+// Our stub hunspell engine makes things a bit flaky.
+PromiseTestUtils.whitelistRejectionsGlobally(/spellCheck is undefined/);
+
 // Enable loading extensions from the user scopes
 Services.prefs.setIntPref("extensions.enabledScopes",
                           AddonManager.SCOPE_PROFILE + AddonManager.SCOPE_USER);
 
 // The test extension uses an insecure update url.
 Services.prefs.setBoolPref(PREF_EM_CHECK_UPDATE_SECURITY, false);
 
 const ID_DICT = "ab-CD@dictionaries.addons.mozilla.org";
--- a/toolkit/mozapps/extensions/test/xpcshell/test_dictionary_webextension.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_dictionary_webextension.js
@@ -4,16 +4,20 @@
 "use strict";
 
 XPCOMUtils.defineLazyServiceGetter(this, "spellCheck",
                                    "@mozilla.org/spellchecker/engine;1", "mozISpellCheckingEngine");
 
 add_task(async function setup() {
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "61", "61");
 
+  // Initialize the URLPreloader so that we can load the built-in
+  // add-ons list, which contains the list of built-in dictionaries.
+  AddonTestUtils.initializeURLPreloader();
+
   await promiseStartupManager();
 });
 
 add_task(async function test_validation() {
   await Assert.rejects(
     promiseInstallWebExtension({
       manifest: {
         applications: {gecko: {id: "en-US-no-dic@dictionaries.mozilla.org"}},