Bug 1457321: Part 3 - Load built-in dictionaries from omnijar rather than scanning directories. r?aswan,ehsan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 27 Apr 2018 13:43:50 -0700
changeset 789285 07d42e3f8ab6b558115c9fc3032ed49091ceb9c2
parent 789284 3c2ebc8c22187977dbf9d94521e4138f34579b6d
child 792313 4be5425862006411c64997f7a9bc8061cad74b85
push id108244
push usermaglione.k@gmail.com
push dateFri, 27 Apr 2018 23:13:05 +0000
reviewersaswan, ehsan
bugs1457321
milestone61.0a1
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;
@@ -643,20 +610,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
@@ -91,18 +91,18 @@ interface mozISpellCheckingEngine : nsIS
    * Add a dictionary with the given language code and file URI.
    */
   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
@@ -50,16 +50,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
   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",
   setTimeout: "resource://gre/modules/Timer.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
@@ -1880,19 +1881,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
@@ -1548,27 +1548,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
@@ -44,16 +44,19 @@ XPCOMUtils.defineLazyModuleGetters(this,
   XPIDatabaseReconcile: "resource://gre/modules/addons/XPIDatabase.jsm",
   XPIInstall: "resource://gre/modules/addons/XPIInstall.jsm",
   verifyBundleSignedState: "resource://gre/modules/addons/XPIInstall.jsm",
 });
 
 XPCOMUtils.defineLazyServiceGetter(this, "aomStartup",
                                    "@mozilla.org/addons/addon-manager-startup;1",
                                    "amIAddonManagerStartup");
+XPCOMUtils.defineLazyServiceGetter(this, "spellCheck",
+                                   "@mozilla.org/spellchecker/engine;1",
+                                   "mozISpellCheckingEngine");
 
 Cu.importGlobalProperties(["URL"]);
 
 const nsIFile = Components.Constructor("@mozilla.org/file/local;1", "nsIFile",
                                        "initWithPath");
 
 const PREF_DB_SCHEMA                  = "extensions.databaseSchema";
 const PREF_XPI_STATE                  = "extensions.xpiState";
@@ -1412,16 +1415,55 @@ var XPIProvider = {
         c.cancel();
       } catch (e) {
         logger.warn("Cancel failed", 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.
@@ -1507,16 +1549,27 @@ var XPIProvider = {
       AddonManagerPrivate.recordTimestamp("XPI_startup_begin");
 
       logger.debug("startup");
       this.runPhase = XPI_STARTING;
       this.installs = new Set();
       this.installLocations = [];
       this.installLocationsByName = {};
 
+      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);
 
       let hasRegistry = ("nsIWindowsRegKey" in Ci);
 
       let enabledScopes = Services.prefs.getIntPref(PREF_EM_ENABLED_SCOPES,
@@ -3049,25 +3102,17 @@ forwardInstallMethods(MutableDirectoryIn
  * This location should point either to a XPI, or a directory in a local build.
  */
 class BuiltInInstallLocation extends DirectoryInstallLocation {
   /**
    * Read the manifest of allowed add-ons and build a mapping between ID and URI
    * for each.
    */
   _readAddons() {
-    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;
-    }
+    let manifest = XPIProvider.builtInAddons;
 
     if (!("system" in manifest)) {
       logger.warn("No list of valid system add-ons found.");
       return;
     }
 
     for (let id of manifest.system) {
       let file = new FileUtils.File(this._directory.path);
--- 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"}},