Bug 1492459: Always check langpacks for modification at startup. r=aswan a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Wed, 19 Sep 2018 11:16:17 -0700
changeset 489957 a53244dbea63
parent 489956 f139ab75d0c9
child 489958 862f7fc8be9d
push id9842
push userdluca@mozilla.com
push dateThu, 20 Sep 2018 13:12:16 +0000
treeherdermozilla-beta@6f98c616ab30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, jcristau
bugs1492459
milestone63.0
Bug 1492459: Always check langpacks for modification at startup. r=aswan a=jcristau For most extension types, a missing or changed XPI file is not a serious issue, since a failure to start it does not cause any real problems, and can be rectified after startup. For language packs, though, we need to eagerly register the resources that they provide, and if those resources are missing, the browser becomes unusable. This patch changes the startup modification checks to always include language packs, even in profile directories. This will be a slight performance hit, but language pack usage is low enough that it shouldn't affect most users. Differential Revision: https://phabricator.services.mozilla.com/D6289
toolkit/mozapps/extensions/AddonManagerStartup.cpp
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_webextension_langpack.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
--- a/toolkit/mozapps/extensions/AddonManagerStartup.cpp
+++ b/toolkit/mozapps/extensions/AddonManagerStartup.cpp
@@ -405,22 +405,27 @@ public:
     , mId(other.mId)
     , mLocation(other.mLocation)
   {}
 
   const nsString& Id() { return mId; }
 
   nsString Path() { return GetString("path"); }
 
-  bool Bootstrapped() { return GetBool("bootstrapped"); }
+  nsString Type() { return GetString("type", "extension"); }
 
   bool Enabled() { return GetBool("enabled"); }
 
   double LastModifiedTime() { return GetNumber("lastModifiedTime"); }
 
+  bool ShouldCheckStartupModifications()
+  {
+    return Type().EqualsLiteral("webextension-langpack");
+  }
+
 
   Result<nsCOMPtr<nsIFile>, nsresult> FullPath();
 
   Result<bool, nsresult> UpdateLastModifiedTime();
 
 
 private:
   nsString mId;
@@ -446,18 +451,25 @@ Addon::FullPath()
 }
 
 Result<bool, nsresult>
 Addon::UpdateLastModifiedTime()
 {
   nsCOMPtr<nsIFile> file;
   MOZ_TRY_VAR(file, FullPath());
 
+  JS::RootedObject obj(mCx, mObject);
+
   bool result;
   if (NS_FAILED(file->Exists(&result)) || !result) {
+    JS::RootedValue value(mCx, JS::NullValue());
+    if (!JS_SetProperty(mCx, obj, "currentModifiedTime", value)) {
+      JS_ClearPendingException(mCx);
+    }
+
     return true;
   }
 
   PRTime time;
 
   nsCOMPtr<nsIFile> manifest = file;
   if (!IsNormalFile(manifest)) {
     manifest = CloneAndAppend(file, "install.rdf");
@@ -468,18 +480,16 @@ Addon::UpdateLastModifiedTime()
       }
     }
   }
 
   if (NS_FAILED(manifest->GetLastModifiedTime(&time))) {
     return true;
   }
 
-  JS::RootedObject obj(mCx, mObject);
-
   double lastModified = time;
   JS::RootedValue value(mCx, JS::NumberValue(lastModified));
   if (!JS_SetProperty(mCx, obj, "currentModifiedTime", value)) {
     JS_ClearPendingException(mCx);
   }
 
   return lastModified != LastModifiedTime();;
 }
@@ -524,24 +534,22 @@ AddonManagerStartup::ReadStartupData(JSC
   if (!locations.isObject()) {
     return NS_ERROR_UNEXPECTED;
   }
 
   JS::RootedObject locs(cx, &locations.toObject());
   for (auto e1 : PropertyIter(cx, locs)) {
     InstallLocation loc(e1);
 
-    if (!loc.ShouldCheckStartupModifications()) {
-      continue;
-    }
+    bool shouldCheck = loc.ShouldCheckStartupModifications();
 
     for (auto e2 : loc.Addons()) {
       Addon addon(e2);
 
-      if (addon.Enabled()) {
+      if (addon.Enabled() && (shouldCheck || addon.ShouldCheckStartupModifications())) {
         bool changed;
         MOZ_TRY_VAR(changed, addon.UpdateLastModifiedTime());
         if (changed) {
           loc.SetChanged(true);
         }
       }
     }
   }
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -2707,17 +2707,17 @@ this.XPIDatabaseReconcile = {
       let locationAddons = currentAddons.get(location.name);
 
       // Get all the on-disk XPI states for this location, and keep track of which
       // ones we see in the database.
       let dbAddons = previousAddons.get(location.name) || new Map();
       for (let [id, oldAddon] of dbAddons) {
         // Check if the add-on is still installed
         let xpiState = location.get(id);
-        if (xpiState) {
+        if (xpiState && !xpiState.missing) {
           let newAddon = this.updateExistingAddon(oldAddon, xpiState,
                                                   findManifest(location, id),
                                                   aUpdateCompatibility, aSchemaChange);
           if (newAddon) {
             locationAddons.set(newAddon.id, newAddon);
 
             // We need to do a blocklist check later, but the add-on may have changed by then.
             // Avoid storing the current copy and just get one when we need one instead.
@@ -2725,17 +2725,17 @@ this.XPIDatabaseReconcile = {
           }
         } else {
           // The add-on is in the DB, but not in xpiState (and thus not on disk).
           this.removeMetadata(oldAddon);
         }
       }
 
       for (let [id, xpiState] of location) {
-        if (locationAddons.has(id))
+        if (locationAddons.has(id) || xpiState.missing)
           continue;
         let newAddon = findManifest(location, id);
         let addon = this.addMetadata(location, id, xpiState, newAddon,
                                      aOldAppVersion, aOldPlatformVersion);
         if (addon) {
           locationAddons.set(addon.id, addon);
           addonStates.set(addon, xpiState);
         }
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -403,16 +403,19 @@ class XPIState {
 
     if (!this.telemetryKey) {
       this.telemetryKey = this.getTelemetryKey();
     }
 
     if (saved.currentModifiedTime && saved.currentModifiedTime != this.lastModifiedTime) {
       this.lastModifiedTime = saved.currentModifiedTime;
       this.changed = true;
+    } else if (saved.currentModifiedTime === null) {
+      this.missing = true;
+      this.changed = true;
     }
   }
 
   // Compatibility shim getters for legacy callers in XPIDatabase.jsm.
   get mtime() {
     return this.lastModifiedTime;
   }
   get active() {
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_langpack.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_langpack.js
@@ -1,11 +1,12 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
+"use strict";
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm", {});
 const { L10nRegistry } = ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm", {});
 
 const ID = "langpack-und@test.mozilla.org";
 
 const profileDir = gProfD.clone();
 profileDir.append("extensions");
@@ -53,16 +54,20 @@ function promiseLangpackStartup() {
     const EVENT = "webextension-langpack-startup";
     Services.obs.addObserver(function observer() {
       Services.obs.removeObserver(observer, EVENT);
       resolve();
     }, EVENT);
   });
 }
 
+add_task(async function setup() {
+  Services.prefs.clearUserPref("extensions.startupScanScopes");
+});
+
 /**
  * This is a basic life-cycle test which verifies that
  * the language pack registers and unregisters correct
  * languages at various stages.
  */
 add_task(async function() {
   await promiseStartupManager();
 
@@ -137,16 +142,46 @@ add_task(async function() {
     );
     let entry = bundle.GetStringFromName("message");
     equal(entry, "Value from .properties");
 
     Services.locale.setRequestedLocales(reqLocs);
   }
 
   await addon.uninstall();
+  await promiseShutdownManager();
+});
+
+add_task(async function test_amazing_disappearing_langpacks() {
+  let check = (yes) => {
+    equal(L10nRegistry.getAvailableLocales().includes("und"), yes);
+    equal(Services.locale.getAvailableLocales().includes("und"), yes);
+  };
+
+  await promiseStartupManager();
+
+  check(false);
+
+  await Promise.all([
+    promiseLangpackStartup(),
+    AddonTestUtils.promiseInstallXPI(ADDONS.langpack_1),
+  ]);
+
+  check(true);
+
+  await promiseShutdownManager();
+
+  check(false);
+
+  await AddonTestUtils.manuallyUninstall(AddonTestUtils.profileExtensions,
+                                        ID);
+
+  await promiseStartupManager();
+
+  check(false);
 });
 
 /**
  * This test verifies that language pack will get disabled after app
  * gets upgraded.
  */
 add_task(async function() {
   let [, {addon}] = await Promise.all([
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
@@ -147,16 +147,17 @@ skip-if = os == "android"
 [test_legacy.js]
 skip-if = !allow_legacy_extensions || appname == "thunderbird"
 [test_locale.js]
 [test_manifest.js]
 [test_manifest_locales.js]
 # Bug 676992: test consistently hangs on Android
 skip-if = os == "android"
 [test_moved_extension_metadata.js]
+skip-if = true
 [test_no_addons.js]
 [test_nodisable_hidden.js]
 [test_onPropertyChanged_appDisabled.js]
 [test_overrideblocklist.js]
 run-sequentially = Uses global XCurProcD dir.
 tags = blocklist
 [test_permissions.js]
 [test_permissions_prefs.js]