Bug 963327 - Improve profile-dependent handling of OS.Constants.Path; r=Yoric
authorGregory Szorc <gps@mozilla.com>
Fri, 24 Jan 2014 11:34:30 -0800
changeset 181435 eec4fc585ddd342c6634a5560710f09d500ba294
parent 181434 59320a8f253677b336d296fa4d35e224c7b3afeb
child 181436 f9c55ef9d0106ff895bb0ff94badcd32666661e6
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs963327
milestone29.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 963327 - Improve profile-dependent handling of OS.Constants.Path; r=Yoric Many properties in OS.Constants.Path are dependent on the profile being available. This patch improves their handling. Previously, we had some repeated and boilerplate code for making OS.Constants.Paths.<prop> a lazy getter. This patch eliminates the boilerplate by iterating over the properties that need to be lazy getters. AppData and UAppData are now lazy getters. test_profiledir.js has been rolled into test_path_constants.js. test_path_constants.js now emits a warning when a comparison doesn't test anything. This should help identify ineffective tests going forward.
toolkit/components/osfile/modules/osfile_async_front.jsm
toolkit/components/osfile/tests/xpcshell/test_path_constants.js
toolkit/components/osfile/tests/xpcshell/test_profiledir.js
toolkit/components/osfile/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/osfile/modules/osfile_async_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_async_front.jsm
@@ -53,49 +53,52 @@ Cu.import("resource://gre/modules/Promis
 
 // The implementation of communications
 Cu.import("resource://gre/modules/osfile/_PromiseWorker.jsm", this);
 
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm", this);
 Cu.import("resource://gre/modules/AsyncShutdown.jsm", this);
 
-// If profileDir is not available, osfile.jsm has been imported before the
-// profile is setup. In this case, make this a lazy getter.
-if (!("profileDir" in SharedAll.Constants.Path)) {
-  Object.defineProperty(SharedAll.Constants.Path, "profileDir", {
-    get: function() {
-      let path = undefined;
-      try {
-        path = Services.dirsvc.get("ProfD", Ci.nsIFile).path;
-        delete SharedAll.Constants.Path.profileDir;
-        SharedAll.Constants.Path.profileDir = path;
-      } catch (ex) {
-        // Ignore errors: profileDir is still not available
-      }
-      return path;
+// It's possible for osfile.jsm to get imported before the profile is
+// set up. In this case, some path constants aren't yet available.
+// Here, we make them lazy loaders.
+
+function lazyPathGetter(constProp, dirKey) {
+  return function () {
+    let path;
+    try {
+      path = Services.dirsvc.get(dirKey, Ci.nsIFile).path;
+      delete SharedAll.Constants.Path[constProp];
+      SharedAll.Constants.Path[constProp] = path;
+    } catch (ex) {
+      // Ignore errors if the value still isn't available. Hopefully
+      // the next access will return it.
     }
-  });
+
+    return path;
+  }
 }
 
-LOG("Checking localProfileDir");
+for (let [constProp, dirKey] of [
+  ["localProfileDir", "ProfLD"],
+  ["profileDir", "ProfD"],
+  ["userApplicationDataDir", "UAppData"],
+  ["winAppDataDir", "AppData"],
+  ["winStartMenuProgsDir", "Progs"],
+  ]) {
 
-if (!("localProfileDir" in SharedAll.Constants.Path)) {
-  Object.defineProperty(SharedAll.Constants.Path, "localProfileDir", {
-    get: function() {
-      let path = undefined;
-      try {
-        path = Services.dirsvc.get("ProfLD", Ci.nsIFile).path;
-        delete SharedAll.Constants.Path.localProfileDir;
-        SharedAll.Constants.Path.localProfileDir = path;
-      } catch (ex) {
-        // Ignore errors: localProfileDir is still not available
-      }
-      return path;
-    }
+  if (constProp in SharedAll.Constants.Path) {
+    continue;
+  }
+
+  LOG("Installing lazy getter for OS.Constants.Path." + constProp +
+      " because it isn't defined and profile may not be loaded.");
+  Object.defineProperty(SharedAll.Constants.Path, constProp, {
+    get: lazyPathGetter(constProp, dirKey),
   });
 }
 
 /**
  * Return a shallow clone of the enumerable properties of an object.
  */
 let clone = SharedAll.clone;
 
--- a/toolkit/components/osfile/tests/xpcshell/test_path_constants.js
+++ b/toolkit/components/osfile/tests/xpcshell/test_path_constants.js
@@ -1,36 +1,63 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-Components.utils.import("resource://gre/modules/osfile.jsm");
-Components.utils.import("resource://gre/modules/Services.jsm");
-Components.utils.import("resource://gre/modules/ctypes.jsm");
+const Cu = Components.utils;
+
+Cu.import("resource://gre/modules/osfile.jsm", this);
+Cu.import("resource://gre/modules/Services.jsm", this);
+Cu.import("resource://gre/modules/ctypes.jsm", this);
+Cu.import("resource://testing-common/AppData.jsm", this);
+
 
 function run_test() {
   run_next_test();
 }
 
 function compare_paths(ospath, key) {
   let file;
   try {
     file = Services.dirsvc.get(key, Components.interfaces.nsIFile);
   } catch(ex) {}
 
   if (file) {
     do_check_true(!!ospath);
     do_check_eq(ospath, file.path);
   } else {
+    do_print("WARNING: " + key + " is not defined. Test may not be testing anything!");
     do_check_false(!!ospath);
   }
 }
 
+// Some path constants aren't set up until the profile is available. This
+// test verifies that behavior.
+add_task(function* test_before_after_profile() {
+  do_check_null(OS.Constants.Path.profileDir);
+  do_check_null(OS.Constants.Path.localProfileDir);
+  do_check_null(OS.Constants.Path.userApplicationDataDir);
+
+  do_get_profile();
+  do_check_true(!!OS.Constants.Path.profileDir);
+  do_check_true(!!OS.Constants.Path.localProfileDir);
+
+  // UAppData is still null because the xpcshell profile doesn't set it up.
+  // This test is mostly here to fail in case behavior of do_get_profile() ever
+  // changes. We want to know if our assumptions no longer hold!
+  do_check_null(OS.Constants.Path.userApplicationDataDir);
+
+  yield makeFakeAppDir();
+  do_check_true(!!OS.Constants.Path.userApplicationDataDir);
+
+  // FUTURE: verify AppData too (bug 964291).
+});
+
 // Test simple paths
 add_task(function() {
   do_check_true(!!OS.Constants.Path.tmpDir);
   do_check_eq(OS.Constants.Path.tmpDir, Services.dirsvc.get("TmpD", Components.interfaces.nsIFile).path);
 
   do_check_true(!!OS.Constants.Path.homeDir);
   do_check_eq(OS.Constants.Path.homeDir, Services.dirsvc.get("Home", Components.interfaces.nsIFile).path);
 
deleted file mode 100644
--- a/toolkit/components/osfile/tests/xpcshell/test_profiledir.js
+++ /dev/null
@@ -1,43 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-Components.utils.import("resource://gre/modules/osfile.jsm");
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-function run_test() {
-  run_next_test();
-}
-
-add_test(function test_initialize_profileDir() {
-  // Profile has not been set up yet, check that "profileDir" isn't either.
-  do_check_false(!!OS.Constants.Path.profileDir);
-  do_check_false(!!OS.Constants.Path.localProfileDir);
-
-  // Set up profile.
-  do_get_profile();
-
-  // Now that profile has been set up, check that "profileDir" is set.
-  do_check_true(!!OS.Constants.Path.profileDir);
-  do_check_eq(OS.Constants.Path.profileDir,
-              Services.dirsvc.get("ProfD", Components.interfaces.nsIFile).path);
-
-  do_check_true(!!OS.Constants.Path.localProfileDir);
-  do_check_eq(OS.Constants.Path.localProfileDir,
-              Services.dirsvc.get("ProfLD", Components.interfaces.nsIFile).path);
-
-  let promise = OS.File.makeDir(OS.Path.join(OS.Constants.Path.profileDir, "foobar"));
-  promise.then(
-    function onSuccess() {
-      do_print("Directory creation succeeded");
-      run_next_test();
-    },
-
-    function onFailure(reason) {
-      do_fail(reason);
-      run_next_test();
-    }
-  );
-});
--- a/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ -7,17 +7,16 @@ tail =
 [test_osfile_async.js]
 [test_osfile_async_append.js]
 [test_osfile_async_bytes.js]
 [test_osfile_async_copy.js]
 [test_osfile_async_flush.js]
 [test_osfile_async_setDates.js]
 [test_removeEmptyDir.js]
 [test_makeDir.js]
-[test_profiledir.js]
 [test_file_URL_conversion.js]
 [test_logging.js]
 [test_creationDate.js]
 [test_exception.js]
 [test_path_constants.js]
 [test_removeDir.js]
 [test_reset.js]
 [test_shutdown.js]