Bug 1388134 - Defer load of OS.Constants.Path members r=gsvelto
☠☠ backed out by 017a4a49c716 ☠ ☠
authorDoug Thayer <dothayer@mozilla.com>
Mon, 01 Oct 2018 16:55:04 +0000
changeset 494760 aeb38f1ace8d5f278d0e436e8dcfc68dc9da07c8
parent 494759 238ec26bef308495ac182b6b66dc5cb6803ea5b4
child 494761 365ac2b9486fdff60e9362012a6bbb13ce80a5c5
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto
bugs1388134
milestone64.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 1388134 - Defer load of OS.Constants.Path members r=gsvelto In bug 1388134 we're lazifying some members of OS.Constants.Path to avoid the extra startup IO. userApplicationDataDir is ripe for being made lazy, except it's read early in CrashManager.jsm. This defers that until it's used, and adjusts the affected tests. Depends on D6079 Differential Revision: https://phabricator.services.mozilla.com/D6080
toolkit/components/crashes/CrashManager.jsm
toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
toolkit/crashreporter/test/unit/test_event_files.js
--- a/toolkit/components/crashes/CrashManager.jsm
+++ b/toolkit/components/crashes/CrashManager.jsm
@@ -108,47 +108,32 @@ function parseAndRemoveField(obj, field)
  *   storeDir (string)
  *     Directory we will use for our data store. This instance will write
  *     data files into the directory specified.
  *
  *   telemetryStoreSizeKey (string)
  *     Telemetry histogram to report store size under.
  */
 var CrashManager = function(options) {
-  for (let k of ["pendingDumpsDir", "submittedDumpsDir", "eventsDirs",
-    "storeDir"]) {
-    if (!(k in options)) {
-      throw new Error("Required key not present in options: " + k);
-    }
-  }
-
   this._log = Log.repository.getLogger("Crashes.CrashManager");
 
   for (let k in options) {
-    let v = options[k];
+    let value = options[k];
 
     switch (k) {
       case "pendingDumpsDir":
-        this._pendingDumpsDir = v;
-        break;
-
       case "submittedDumpsDir":
-        this._submittedDumpsDir = v;
-        break;
-
       case "eventsDirs":
-        this._eventsDirs = v;
+      case "storeDir":
+        let key = "_" + k;
+        delete this[key];
+        Object.defineProperty(this, key, {value});
         break;
-
-      case "storeDir":
-        this._storeDir = v;
-        break;
-
       case "telemetryStoreSizeKey":
-        this._telemetryStoreSizeKey = v;
+        this._telemetryStoreSizeKey = value;
         break;
 
       default:
         throw new Error("Unknown property in options: " + k);
     }
   }
 
   // Promise for in-progress aggregation operation. We store it on the
@@ -216,16 +201,57 @@ this.CrashManager.prototype = Object.fre
   // The following are return codes for individual event file processing.
   // File processed OK.
   EVENT_FILE_SUCCESS: "ok",
   // The event appears to be malformed.
   EVENT_FILE_ERROR_MALFORMED: "malformed",
   // The type of event is unknown.
   EVENT_FILE_ERROR_UNKNOWN_EVENT: "unknown-event",
 
+  _lazyGetDir(field, path, leaf) {
+    delete this[field];
+    let value = OS.Path.join(path, leaf);
+    Object.defineProperty(this, field, { value });
+    return value;
+  },
+
+  get _crDir() {
+    return this._lazyGetDir("_crDir",
+                            OS.Constants.Path.userApplicationDataDir,
+                            "Crash Reports");
+  },
+
+  get _storeDir() {
+    return this._lazyGetDir("_storeDir",
+                            OS.Constants.Path.profileDir,
+                            "crashes");
+  },
+
+  get _pendingDumpsDir() {
+    return this._lazyGetDir("_pendingDumpsDir",
+                            this._crDir,
+                            "pending");
+  },
+
+  get _submittedDumpsDir() {
+    return this._lazyGetDir("_submittedDumpsDir",
+                            this._crDir,
+                            "submitted");
+  },
+
+  get _eventsDirs() {
+    delete this._eventsDirs;
+    let value = [
+      OS.Path.join(this._crDir, "events"),
+      OS.Path.join(this._storeDir, "events"),
+    ];
+    Object.defineProperty(this, "_eventsDirs", { value });
+    return value;
+  },
+
   /**
    * Obtain a list of all dumps pending upload.
    *
    * The returned value is a promise that resolves to an array of objects
    * on success. Each element in the array has the following properties:
    *
    *   id (string)
    *      The ID of the crash (a UUID).
@@ -1425,25 +1451,17 @@ CrashRecord.prototype = Object.freeze({
  * CrashManager is likely only ever instantiated once per application lifetime.
  * The main reason it's implemented as a reusable type is to facilitate testing.
  */
 XPCOMUtils.defineLazyGetter(this.CrashManager, "Singleton", function() {
   if (gCrashManager) {
     return gCrashManager;
   }
 
-  let crPath = OS.Path.join(OS.Constants.Path.userApplicationDataDir,
-                            "Crash Reports");
-  let storePath = OS.Path.join(OS.Constants.Path.profileDir, "crashes");
-
   gCrashManager = new CrashManager({
-    pendingDumpsDir: OS.Path.join(crPath, "pending"),
-    submittedDumpsDir: OS.Path.join(crPath, "submitted"),
-    eventsDirs: [OS.Path.join(crPath, "events"), OS.Path.join(storePath, "events")],
-    storeDir: storePath,
     telemetryStoreSizeKey: "CRASH_STORE_COMPRESSED_BYTES",
   });
 
   // Automatically aggregate event files shortly after startup. This
   // ensures it happens with some frequency.
   //
   // There are performance considerations here. While this is doing
   // work and could negatively impact performance, the amount of work
--- a/toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
+++ b/toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
@@ -32,17 +32,17 @@ add_task(async function test_constructor
     storeDir: "/baz",
   });
   Assert.ok(m, "CrashManager can be created.");
 });
 
 add_task(async function test_constructor_invalid() {
   Assert.throws(() => {
     new CrashManager({foo: true});
-  }, /Required key not present in options/);
+  }, /Unknown property in options/);
 });
 
 add_task(async function test_get_manager() {
   let m = await getManager();
   Assert.ok(m, "CrashManager obtained.");
 
   await m.createDummyDump(true);
   await m.createDummyDump(false);
--- a/toolkit/crashreporter/test/unit/test_event_files.js
+++ b/toolkit/crashreporter/test/unit/test_event_files.js
@@ -23,17 +23,17 @@ add_task(async function test_main_proces
         let scope = {};
         ChromeUtils.import("resource://gre/modules/TelemetryController.jsm", scope);
         scope.TelemetryController.testSetup();
         crashType = CrashTestUtils.CRASH_MOZ_CRASH;
         crashReporter.annotateCrashReport("ShutdownProgress", "event-test");
       },
       (minidump, extra) => {
         basename = minidump.leafName;
-        cm._eventsDirs = [getEventDir()];
+        Object.defineProperty(cm, "_eventsDirs", {value: [getEventDir()]});
         cm.aggregateEventsFiles().then(resolve, reject);
       },
       true);
 
   });
   Assert.equal(count, 1, "A single crash event file was seen.");
   let crashes = await cm.getCrashes();
   Assert.equal(crashes.length, 1);