Backed out changeset 21793ea94b09 (bug 875562) for ASAN xpcshell failures

services/healthreport/docs/dataformat.rst

@@ -1024,25 +1024,6 @@ org.mozilla.crashes.crashes
 
 This measurement contains a historical record of application crashes.
 
-Version 2
-^^^^^^^^^
-
-The switch to version 2 coincides with the introduction of the
-:ref:`crashes_crashmanager`, which provides a more robust source of
-crash data.
-
-This measurement will be reported on each day there was a crash. The
-following fields may be present in each record:
-
-mainCrash
-   The number of main process crashes that occurred on the given day.
-
-Yes, version 2 does not track submissions like version 1. It is very
-likely submissions will be re-added later.
-
-Also absent from version 2 are plugin crashes and hangs. These will be
-re-added, likely in version 3.
-
 Version 1
 ^^^^^^^^^
 
@@ -1057,10 +1038,10 @@ submitted
 Notes
 ^^^^^
 
-Main process crashes are typically submitted immediately after they
-occur (by checking a box in the crash reporter, which should appear
-automatically after a crash). If the crash reporter submits the crash
-successfully, we get a submitted crash. Else, we leave it as pending.
+Crashes are typically submitted immediately after they occur (by checking
+a box in the crash reporter, which should appear automatically after a
+crash). If the crash reporter submits the crash successfully, we get a
+submitted crash. Else, we leave it as pending.
 
 A pending crash does not mean it will eventually be submitted.
 
@@ -1081,10 +1062,6 @@ Example
       "_v": 1,
       "pending": 1,
       "submitted": 2
-    },
-    "org.mozilla.crashes.crashes": {
-      "_v": 2,
-      "mainCrash": 2
     }
 
 org.mozilla.healthreport.submissions

services/healthreport/providers.jsm

@@ -993,11 +993,11 @@ AddonsProvider.prototype = Object.freeze({
 });
 
 
-function DailyCrashesMeasurement1() {
+function DailyCrashesMeasurement() {
   Metrics.Measurement.call(this);
 }
 
-DailyCrashesMeasurement1.prototype = Object.freeze({
+DailyCrashesMeasurement.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "crashes",
@@ -1009,26 +1009,8 @@ DailyCrashesMeasurement1.prototype = Object.freeze({
   },
 });
 
-function DailyCrashesMeasurement2() {
-  Metrics.Measurement.call(this);
-}
-
-DailyCrashesMeasurement2.prototype = Object.freeze({
-  __proto__: Metrics.Measurement.prototype,
-
-  name: "crashes",
-  version: 2,
-
-  fields: {
-    mainCrash: DAILY_LAST_NUMERIC_FIELD,
-  },
-});
-
 this.CrashesProvider = function () {
   Metrics.Provider.call(this);
-
-  // So we can unit test.
-  this._manager = Services.crashmanager;
 };
 
 CrashesProvider.prototype = Object.freeze({
@@ -1036,37 +1018,161 @@ CrashesProvider.prototype = Object.freeze({
 
   name: "org.mozilla.crashes",
 
-  measurementTypes: [
-    DailyCrashesMeasurement1,
-    DailyCrashesMeasurement2,
-  ],
+  measurementTypes: [DailyCrashesMeasurement],
 
   pullOnly: true,
 
-  collectDailyData: function () {
+  collectConstantData: function () {
     return this.storage.enqueueTransaction(this._populateCrashCounts.bind(this));
   },
 
   _populateCrashCounts: function () {
-    this._log.info("Grabbing crash counts from crash manager.");
-    let crashCounts = yield this._manager.getCrashCountsByDay();
-    let fields = {
-      "main-crash": "mainCrash",
-    };
+    let now = new Date();
+    let service = new CrashDirectoryService();
 
-    let m = this.getMeasurement("crashes", 2);
+    let pending = yield service.getPendingFiles();
+    let submitted = yield service.getSubmittedFiles();
 
-    for (let [day, types] of crashCounts) {
-      let date = Metrics.daysToDate(day);
-      for (let [type, count] of types) {
-        if (!(type in fields)) {
-          this._log.warn("Unknown crash type encountered: " + type);
-          continue;
-        }
+    function getAgeLimit() {
+      return 0;
+    }
 
-        yield m.setDailyLastNumeric(fields[type], count, date);
+    let lastCheck = yield this.getState("lastCheck");
+    if (!lastCheck) {
+      lastCheck = getAgeLimit();
+    } else {
+      lastCheck = parseInt(lastCheck, 10);
+      if (Number.isNaN(lastCheck)) {
+        lastCheck = getAgeLimit();
       }
     }
+
+    let m = this.getMeasurement("crashes", 1);
+
+    // Aggregate counts locally to avoid excessive storage interaction.
+    let counts = {
+      pending: new Metrics.DailyValues(),
+      submitted: new Metrics.DailyValues(),
+    };
+
+    // FUTURE detect mtimes in the future and react more intelligently.
+    for (let filename in pending) {
+      let modified = pending[filename].modified;
+
+      if (modified.getTime() < lastCheck) {
+        continue;
+      }
+
+      counts.pending.appendValue(modified, 1);
+    }
+
+    for (let filename in submitted) {
+      let modified = submitted[filename].modified;
+
+      if (modified.getTime() < lastCheck) {
+        continue;
+      }
+
+      counts.submitted.appendValue(modified, 1);
+    }
+
+    for (let [date, values] in counts.pending) {
+      yield m.incrementDailyCounter("pending", date, values.length);
+    }
+
+    for (let [date, values] in counts.submitted) {
+      yield m.incrementDailyCounter("submitted", date, values.length);
+    }
+
+    yield this.setState("lastCheck", "" + now.getTime());
+  },
+});
+
+
+/**
+ * Helper for interacting with the crashes directory.
+ *
+ * FUTURE Extract to JSM alongside crashreporter. Use in about:crashes.
+ */
+this.CrashDirectoryService = function () {
+  let base = Cc["@mozilla.org/file/directory_service;1"]
+               .getService(Ci.nsIProperties)
+               .get("UAppData", Ci.nsIFile);
+
+  let cr = base.clone();
+  cr.append("Crash Reports");
+
+  let submitted = cr.clone();
+  submitted.append("submitted");
+
+  let pending = cr.clone();
+  pending.append("pending");
+
+  this._baseDir = base.path;
+  this._submittedDir = submitted.path;
+  this._pendingDir = pending.path;
+};
+
+CrashDirectoryService.prototype = Object.freeze({
+  RE_SUBMITTED_FILENAME: /^bp-.+\.txt$/,
+  RE_PENDING_FILENAME: /^.+\.dmp$/,
+
+  getPendingFiles: function () {
+    return this._getDirectoryEntries(this._pendingDir,
+                                     this.RE_PENDING_FILENAME);
+  },
+
+  getSubmittedFiles: function () {
+    return this._getDirectoryEntries(this._submittedDir,
+                                     this.RE_SUBMITTED_FILENAME);
+  },
+
+  _getDirectoryEntries: function (path, re) {
+    let files = {};
+
+    return Task.spawn(function iterateDirectory() {
+      // If the directory doesn't exist, exit immediately. Else, re-throw
+      // any errors.
+      try {
+        yield OS.File.stat(path);
+      } catch (ex if ex instanceof OS.File.Error) {
+        if (ex.becauseNoSuchFile) {
+          throw new Task.Result({});
+        }
+
+        throw ex;
+      }
+
+      let iterator = new OS.File.DirectoryIterator(path);
+
+      try {
+        while (true) {
+          let entry;
+          try {
+            entry = yield iterator.next();
+          } catch (ex if ex == StopIteration) {
+            break;
+          }
+
+          if (!entry.name.match(re)) {
+            continue;
+          }
+
+          let info = yield OS.File.stat(entry.path);
+
+          files[entry.name] = {
+            // Last modified should be adequate, because crash files aren't
+            // modified after they're first written.
+            modified: info.lastModificationDate,
+            size: info.size,
+          };
+        }
+
+        throw new Task.Result(files);
+      } finally {
+        iterator.close();
+      }
+    });
   },
 });
 

services/healthreport/tests/xpcshell/test_provider_crashes.js

@@ -8,25 +8,79 @@ const {utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Metrics.jsm");
 Cu.import("resource://gre/modules/services/healthreport/providers.jsm");
-Cu.import("resource://testing-common/AppData.jsm");
 Cu.import("resource://testing-common/services/healthreport/utils.jsm");
-Cu.import("resource://testing-common/CrashManagerTest.jsm");
+Cu.import("resource://testing-common/AppData.jsm");
+
+
+const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000;
 
 
 function run_test() {
   run_next_test();
 }
 
-add_task(function* init() {
-  do_get_profile();
+// run_test() needs to finish synchronously, so we do async init here.
+add_task(function test_init() {
   yield makeFakeAppDir();
 });
 
-add_task(function test_constructor() {
-  let provider = new CrashesProvider();
+let gPending = {};
+let gSubmitted = {};
+
+add_task(function test_directory_service() {
+  let d = new CrashDirectoryService();
+
+  let entries = yield d.getPendingFiles();
+  do_check_eq(typeof(entries), "object");
+  do_check_eq(Object.keys(entries).length, 0);
+
+  entries = yield d.getSubmittedFiles();
+  do_check_eq(typeof(entries), "object");
+  do_check_eq(Object.keys(entries).length, 0);
+
+  let now = new Date();
+
+  // We lose granularity when writing to filesystem.
+  now.setUTCMilliseconds(0);
+  let dates = [];
+  for (let i = 0; i < 10; i++) {
+    dates.push(new Date(now.getTime() - i * MILLISECONDS_PER_DAY));
+  }
+
+  let pending = {};
+  let submitted = {};
+  for (let date of dates) {
+    pending[createFakeCrash(false, date)] = date;
+    submitted[createFakeCrash(true, date)] = date;
+  }
+
+  entries = yield d.getPendingFiles();
+  do_check_eq(Object.keys(entries).length, Object.keys(pending).length);
+  for (let id in pending) {
+    let filename = id + ".dmp";
+    do_check_true(filename in entries);
+    do_check_eq(entries[filename].modified.getTime(), pending[id].getTime());
+  }
+
+  entries = yield d.getSubmittedFiles();
+  do_check_eq(Object.keys(entries).length, Object.keys(submitted).length);
+  for (let id in submitted) {
+    let filename = "bp-" + id + ".txt";
+    do_check_true(filename in entries);
+    do_check_eq(entries[filename].modified.getTime(), submitted[id].getTime());
+  }
+
+  gPending = pending;
+  gSubmitted = submitted;
 });
 
-add_task(function* test_init() {
+add_test(function test_constructor() {
+  let provider = new CrashesProvider();
+
+  run_next_test();
+});
+
+add_task(function test_init() {
   let storage = yield Metrics.Storage("init");
   let provider = new CrashesProvider();
   yield provider.init(storage);
@@ -35,52 +89,66 @@ add_task(function* test_init() {
   yield storage.close();
 });
 
-add_task(function* test_collect() {
+add_task(function test_collect() {
   let storage = yield Metrics.Storage("collect");
   let provider = new CrashesProvider();
   yield provider.init(storage);
 
-  // Install custom manager so we don't interfere with other tests.
-  let manager = yield getManager();
-  provider._manager = manager;
+  // FUTURE Don't rely on state from previous test.
+  yield provider.collectConstantData();
 
-  let day1 = new Date(2014, 0, 1, 0, 0, 0);
-  let day2 = new Date(2014, 0, 3, 0, 0, 0);
-
-  // FUTURE Bug 982836 CrashManager will grow public APIs for adding crashes.
-  // Switch to that here.
-  let store = yield manager._getStore();
-  store.addMainProcessCrash("id1", day1);
-  store.addMainProcessCrash("id2", day1);
-  store.addMainProcessCrash("id3", day2);
-
-  // Flush changes (this may not be needed but it doesn't hurt).
-  yield store.save();
-
-  yield provider.collectDailyData();
-
-  let m = provider.getMeasurement("crashes", 2);
+  let m = provider.getMeasurement("crashes", 1);
   let values = yield m.getValues();
-  do_check_eq(values.days.size, 2);
-  do_check_true(values.days.hasDay(day1));
-  do_check_true(values.days.hasDay(day2));
+  do_check_eq(values.days.size, Object.keys(gPending).length);
+  for each (let date in gPending) {
+    do_check_true(values.days.hasDay(date));
 
-  let value = values.days.getDay(day1);
-  do_check_true(value.has("mainCrash"));
-  do_check_eq(value.get("mainCrash"), 2);
+    let value = values.days.getDay(date);
+    do_check_true(value.has("pending"));
+    do_check_true(value.has("submitted"));
+    do_check_eq(value.get("pending"), 1);
+    do_check_eq(value.get("submitted"), 1);
+  }
 
-  value = values.days.getDay(day2);
-  do_check_eq(value.get("mainCrash"), 1);
+  let currentState = yield provider.getState("lastCheck");
+  do_check_eq(typeof(currentState), "string");
+  do_check_true(currentState.length > 0);
+  let lastState = currentState;
 
-  // Check that adding a new crash increments counter on next collect.
-  store = yield manager._getStore();
-  store.addMainProcessCrash("id4", day2);
-  yield store.save();
-
-  yield provider.collectDailyData();
+  // If we collect again, we should get no new data.
+  yield provider.collectConstantData();
   values = yield m.getValues();
-  value = values.days.getDay(day2);
-  do_check_eq(value.get("mainCrash"), 2);
+  for each (let date in gPending) {
+    let day = values.days.getDay(date);
+    do_check_eq(day.get("pending"), 1);
+    do_check_eq(day.get("submitted"), 1);
+  }
+
+  currentState = yield provider.getState("lastCheck");
+  do_check_neq(currentState, lastState);
+  do_check_true(currentState > lastState);
+
+  let now = new Date();
+  let tomorrow = new Date(now.getTime() + MILLISECONDS_PER_DAY);
+  let yesterday = new Date(now.getTime() - MILLISECONDS_PER_DAY);
+
+  createFakeCrash(false, yesterday);
+
+  // Create multiple to test that multiple are handled properly.
+  createFakeCrash(false, tomorrow);
+  createFakeCrash(false, tomorrow);
+  createFakeCrash(false, tomorrow);
+
+  yield provider.collectConstantData();
+  values = yield m.getValues();
+  do_check_eq(values.days.size, 11);
+  do_check_eq(values.days.getDay(tomorrow).get("pending"), 3);
+
+  for each (let date in gPending) {
+    let day = values.days.getDay(date);
+    do_check_eq(day.get("pending"), 1);
+    do_check_eq(day.get("submitted"), 1);
+  }
 
   yield provider.shutdown();
   yield storage.close();

toolkit/components/crashes/CrashManager.jsm

@@ -516,14 +516,6 @@ this.CrashManager.prototype = Object.freeze({
       return store.crashes;
     }.bind(this));
   },
-
-  getCrashCountsByDay: function () {
-    return Task.spawn(function* () {
-      let store = yield this._getStore();
-
-      return store._countsByDay;
-    }.bind(this));
-  },
 });
 
 let gCrashManager;

toolkit/components/crashes/docs/index.rst

@@ -1,5 +1,3 @@
-.. _crashes_crashmanager:
-
 =============
 Crash Manager
 =============