Bug 1389344 - Avoid checking for distribution.ini existence on every startup. r=mkaply
We can do this check only once after an update, and cache the value until the next app update.
MozReview-Commit-ID: 1yxiKnHIAi2
--- a/browser/components/distribution.js
+++ b/browser/components/distribution.js
@@ -7,55 +7,87 @@ this.EXPORTED_SYMBOLS = [ "DistributionC
var Ci = Components.interfaces;
var Cc = Components.classes;
var Cr = Components.results;
var Cu = Components.utils;
const DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC =
"distribution-customization-complete";
+const PREF_CACHED_FILE_EXISTENCE = "distribution.iniFile.exists.value";
+const PREF_CACHED_FILE_APPVERSION = "distribution.iniFile.exists.appversion";
+
+Cu.import("resource://gre/modules/AppConstants.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
"resource://gre/modules/Preferences.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
this.DistributionCustomizer = function DistributionCustomizer() {
- // For parallel xpcshell testing purposes allow loading the distribution.ini
- // file from the profile folder through an hidden pref.
- let loadFromProfile = Services.prefs.getBoolPref("distribution.testing.loadFromProfile", false);
- let dirSvc = Cc["@mozilla.org/file/directory_service;1"].
- getService(Ci.nsIProperties);
- try {
- let iniFile = loadFromProfile ? dirSvc.get("ProfD", Ci.nsIFile)
- : dirSvc.get("XREAppDist", Ci.nsIFile);
- if (loadFromProfile) {
- iniFile.leafName = "distribution";
- }
- iniFile.append("distribution.ini");
- if (iniFile.exists())
- this._iniFile = iniFile;
- } catch (ex) {}
}
DistributionCustomizer.prototype = {
- _iniFile: null,
+ get _iniFile() {
+ // For parallel xpcshell testing purposes allow loading the distribution.ini
+ // file from the profile folder through an hidden pref.
+ let loadFromProfile = Services.prefs.getBoolPref("distribution.testing.loadFromProfile", false);
+ let dirSvc = Cc["@mozilla.org/file/directory_service;1"].
+ getService(Ci.nsIProperties);
+
+ let iniFile;
+ try {
+ iniFile = loadFromProfile ? dirSvc.get("ProfD", Ci.nsIFile)
+ : dirSvc.get("XREAppDist", Ci.nsIFile);
+ if (loadFromProfile) {
+ iniFile.leafName = "distribution";
+ }
+ iniFile.append("distribution.ini");
+ } catch (ex) {}
+
+ this.__defineGetter__("_iniFile", () => iniFile);
+ return iniFile;
+ },
+
+ get _hasDistributionIni() {
+ if (Services.prefs.prefHasUserValue(PREF_CACHED_FILE_EXISTENCE)) {
+ let knownForVersion = Services.prefs.getStringPref(PREF_CACHED_FILE_APPVERSION, "unknown");
+ if (knownForVersion == AppConstants.MOZ_APP_VERSION) {
+ return Services.prefs.getBoolPref(PREF_CACHED_FILE_EXISTENCE);
+ }
+ }
+
+ let fileExists = this._iniFile.exists();
+ Services.prefs.setBoolPref(PREF_CACHED_FILE_EXISTENCE, fileExists);
+ Services.prefs.setStringPref(PREF_CACHED_FILE_APPVERSION, AppConstants.MOZ_APP_VERSION);
+
+ this.__defineGetter__("_hasDistributionIni", () => fileExists);
+ return fileExists;
+ },
get _ini() {
let ini = null;
try {
- if (this._iniFile) {
+ if (this._hasDistributionIni) {
ini = Cc["@mozilla.org/xpcom/ini-parser-factory;1"].
getService(Ci.nsIINIParserFactory).
createINIParser(this._iniFile);
}
} catch (e) {
- // Unable to parse INI.
- Cu.reportError("Unable to parse distribution.ini");
+ if (e.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
+ // We probably had cached the file existence as true,
+ // but it no longer exists. We could set the new cache
+ // value here, but let's just invalidate the cache and
+ // let it be cached by a single code path on the next check.
+ Services.prefs.clearUserPref(PREF_CACHED_FILE_EXISTENCE);
+ } else {
+ // Unable to parse INI.
+ Cu.reportError("Unable to parse distribution.ini");
+ }
}
this.__defineGetter__("_ini", () => ini);
return this._ini;
},
get _locale() {
const locale = Services.locale.getRequestedLocale() || "en-US";
this.__defineGetter__("_locale", () => locale);
--- a/browser/components/tests/unit/test_distribution.js
+++ b/browser/components/tests/unit/test_distribution.js
@@ -81,16 +81,17 @@ function run_test() {
do_register_cleanup(function() {
// Remove the distribution dir, even if the test failed, otherwise all
// next tests will use it.
let distDir = gProfD.clone();
distDir.append("distribution");
distDir.remove(true);
Assert.ok(!distDir.exists());
+ Services.prefs.clearUserPref("distribution.testing.loadFromProfile");
});
add_task(async function() {
// Force distribution.
let glue = Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIObserver)
glue.observe(null, TOPIC_BROWSERGLUE_TEST, TOPICDATA_DISTRIBUTION_CUSTOMIZATION);
var defaultBranch = Services.prefs.getDefaultBranch(null);
new file mode 100644
--- /dev/null
+++ b/browser/components/tests/unit/test_distribution_cachedexistence.js
@@ -0,0 +1,118 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that DistributionCustomizer correctly caches the existence
+ * of the distribution.ini file and just rechecks it after a version
+ * update.
+ */
+
+const PREF_CACHED_FILE_EXISTENCE = "distribution.iniFile.exists.value";
+const PREF_CACHED_FILE_APPVERSION = "distribution.iniFile.exists.appversion";
+const PREF_LOAD_FROM_PROFILE = "distribution.testing.loadFromProfile";
+
+const gTestDir = do_get_cwd();
+
+Cu.import("resource://gre/modules/AppConstants.jsm");
+
+add_task(async function() {
+ // Start with a clean slate of the prefs that control this feature.
+ Services.prefs.clearUserPref(PREF_CACHED_FILE_APPVERSION);
+ Services.prefs.clearUserPref(PREF_CACHED_FILE_EXISTENCE);
+ setupTest();
+
+ let {DistributionCustomizer} = Cu.import("resource:///modules/distribution.js");
+ let distribution = new DistributionCustomizer();
+
+ copyDistributionToProfile();
+
+ // Check that checking for distribution.ini returns the right value and sets up
+ // the cached prefs.
+ let exists = distribution._hasDistributionIni;
+
+ Assert.ok(exists);
+ Assert.equal(Services.prefs.getBoolPref(PREF_CACHED_FILE_EXISTENCE, undefined),
+ true);
+ Assert.equal(Services.prefs.getStringPref(PREF_CACHED_FILE_APPVERSION, "unknown"),
+ AppConstants.MOZ_APP_VERSION);
+
+ // Check that calling _hasDistributionIni again will use the cached value. We do
+ // this by deleting the file and expecting it to still return true instead of false.
+ // Also, we need to delete _hasDistributionIni from the object because the getter
+ // was replaced with a stored value.
+ deleteDistribution();
+ delete distribution._hasDistributionIni;
+
+ exists = distribution._hasDistributionIni;
+ Assert.ok(exists);
+
+ // Now let's invalidate the PREF_CACHED_FILE_EXISTENCE pref to make sure the
+ // value gets recomputed correctly.
+ Services.prefs.clearUserPref(PREF_CACHED_FILE_EXISTENCE);
+ delete distribution._hasDistributionIni;
+ exists = distribution._hasDistributionIni;
+
+ // It now should return false, as well as storing false in the pref.
+ Assert.ok(!exists);
+ Assert.equal(Services.prefs.getBoolPref(PREF_CACHED_FILE_EXISTENCE, undefined),
+ false);
+
+ // Check now that it will use the new cached value instead of returning true in
+ // the presence of the file.
+ copyDistributionToProfile();
+ delete distribution._hasDistributionIni;
+ exists = distribution._hasDistributionIni;
+
+ Assert.ok(!exists);
+
+ // Now let's do the same, but invalidating the App Version, as if a version
+ // update occurred.
+ Services.prefs.setStringPref(PREF_CACHED_FILE_APPVERSION, "older version");
+ delete distribution._hasDistributionIni;
+ exists = distribution._hasDistributionIni;
+
+ Assert.ok(exists);
+ Assert.equal(Services.prefs.getBoolPref(PREF_CACHED_FILE_EXISTENCE, undefined),
+ true);
+ Assert.equal(Services.prefs.getStringPref(PREF_CACHED_FILE_APPVERSION, "unknown"),
+ AppConstants.MOZ_APP_VERSION);
+});
+
+
+/*
+ * Helper functions
+ */
+function copyDistributionToProfile() {
+ // Copy distribution.ini file to the profile dir.
+ let distroDir = gProfD.clone();
+ distroDir.leafName = "distribution";
+ let iniFile = distroDir.clone();
+ iniFile.append("distribution.ini");
+ if (iniFile.exists()) {
+ iniFile.remove(false);
+ print("distribution.ini already exists, did some test forget to cleanup?");
+ }
+
+ let testDistributionFile = gTestDir.clone();
+ testDistributionFile.append("distribution.ini");
+ testDistributionFile.copyTo(distroDir, "distribution.ini");
+ Assert.ok(testDistributionFile.exists());
+}
+
+function deleteDistribution() {
+ let distroDir = gProfD.clone();
+ distroDir.leafName = "distribution";
+ let iniFile = distroDir.clone();
+ iniFile.append("distribution.ini");
+ iniFile.remove(false);
+}
+
+function setupTest() {
+ // Set special pref to load distribution.ini from the profile folder.
+ Services.prefs.setBoolPref(PREF_LOAD_FROM_PROFILE, true);
+}
+
+do_register_cleanup(function() {
+ deleteDistribution();
+ Services.prefs.clearUserPref(PREF_LOAD_FROM_PROFILE);
+});
--- a/browser/components/tests/unit/xpcshell.ini
+++ b/browser/components/tests/unit/xpcshell.ini
@@ -1,10 +1,11 @@
[DEFAULT]
head = head.js
firefox-appdir = browser
skip-if = toolkit == 'android'
support-files =
distribution.ini
data/engine-de-DE.xml
+[test_browserGlue_migration_loop_cleanup.js]
[test_distribution.js]
-[test_browserGlue_migration_loop_cleanup.js]
+[test_distribution_cachedexistence.js]