Bug 875562 - Part 9: Change Health Report to pull from crashes manager; r=bsmedberg, r=rnewman
authorGregory Szorc <gps@mozilla.com>
Thu, 30 Jan 2014 16:48:52 -0800
changeset 190722 4496b6e98cf63b7ca336d53a5a9ed34d2c7637db
parent 190721 436a9dfc6cb498b6b7ba26e76700ccf8a7504d8e
child 190723 e427640a8060984f23da7105a73aae73e8997e73
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg, rnewman
bugs875562
milestone30.0a1
Bug 875562 - Part 9: Change Health Report to pull from crashes manager; r=bsmedberg, r=rnewman This patch changes Firefox Health Report to pull crash counts from the newly-implemented crash manager. The crash manager is now the canonical source of crash data, so all the code for reading crash dump files from disk has been removed. This regresses the collection capability of Firefox Health Report by removing plugin crashes and hangs from the reported values. This regression is intended to be temporary and a follow-up bug will be created to add plugin process event recording to the crash events system. This regression has been justified because the old crash reporting mechanism was severely flawed and wasn't sending accurate data (not all crashes were being saved to dumps and some dumps would be deleted).
services/healthreport/HealthReportComponents.manifest
services/healthreport/docs/dataformat.rst
services/healthreport/moz.build
services/healthreport/providers.jsm
services/healthreport/tests/xpcshell/test_provider_crashes.js
services/healthreport/tests/xpcshell/xpcshell.ini
toolkit/components/crashes/CrashManager.jsm
toolkit/components/crashes/docs/index.rst
--- a/services/healthreport/HealthReportComponents.manifest
+++ b/services/healthreport/HealthReportComponents.manifest
@@ -1,12 +1,14 @@
 # Register Firefox Health Report providers.
 category healthreport-js-provider-default AddonsProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default AppInfoProvider resource://gre/modules/HealthReport.jsm
+#ifdef MOZ_CRASHREPORTER
 category healthreport-js-provider-default CrashesProvider resource://gre/modules/HealthReport.jsm
+#endif
 category healthreport-js-provider-default HealthReportProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default PlacesProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default ProfileMetadataProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default SearchesProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default SessionsProvider resource://gre/modules/HealthReport.jsm
 category healthreport-js-provider-default SysInfoProvider resource://gre/modules/HealthReport.jsm
 
 # No Aurora or Beta providers yet; use the categories
--- a/services/healthreport/docs/dataformat.rst
+++ b/services/healthreport/docs/dataformat.rst
@@ -1019,34 +1019,53 @@ Example
       ]
     }
 
 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
 ^^^^^^^^^
 
 This measurement will be reported on each day there was a crash. The
 following properties are reported:
 
 pending
     The number of crash reports that haven't been submitted.
 submitted
     The number of crash reports that were submitted.
 
 Notes
 ^^^^^
 
-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.
+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.
 
 A pending crash does not mean it will eventually be submitted.
 
 Pending crash reports can be submitted post-crash by going to
 about:crashes.
 
 If a pending crash is submitted via about:crashes, the submitted count
 increments but the pending count does not decrement. This is because FHR
@@ -1057,16 +1076,20 @@ Example
 ^^^^^^^
 
 ::
 
     "org.mozilla.crashes.crashes": {
       "_v": 1,
       "pending": 1,
       "submitted": 2
+    },
+    "org.mozilla.crashes.crashes": {
+      "_v": 2,
+      "mainCrash": 2
     }
 
 org.mozilla.healthreport.submissions
 ------------------------------------
 
 This measurement contains a history of FHR's own data submission activity.
 It was added in Firefox 23 in early May 2013.
 
--- a/services/healthreport/moz.build
+++ b/services/healthreport/moz.build
@@ -3,11 +3,11 @@
 # 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/.
 
 SPHINX_TREES['healthreport'] = 'docs'
 
 TEST_DIRS += ['tests']
 
-EXTRA_COMPONENTS += [
+EXTRA_PP_COMPONENTS += [
     'HealthReportComponents.manifest',
 ]
--- a/services/healthreport/providers.jsm
+++ b/services/healthreport/providers.jsm
@@ -14,18 +14,19 @@
 
 "use strict";
 
 #ifndef MERGED_COMPARTMENT
 
 this.EXPORTED_SYMBOLS = [
   "AddonsProvider",
   "AppInfoProvider",
-  "CrashDirectoryService",
+#ifdef MOZ_CRASHREPORTER
   "CrashesProvider",
+#endif
   "HealthReportProvider",
   "PlacesProvider",
   "SearchesProvider",
   "SessionsProvider",
   "SysInfoProvider",
 ];
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
@@ -987,199 +988,96 @@ AddonsProvider.prototype = Object.freeze
     }
 
     data.counts["plugin"] = pluginTags.length;
 
     return data;
   },
 });
 
+#ifdef MOZ_CRASHREPORTER
 
-function DailyCrashesMeasurement() {
+function DailyCrashesMeasurement1() {
   Metrics.Measurement.call(this);
 }
 
-DailyCrashesMeasurement.prototype = Object.freeze({
+DailyCrashesMeasurement1.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "crashes",
   version: 1,
 
   fields: {
     pending: DAILY_COUNTER_FIELD,
     submitted: DAILY_COUNTER_FIELD,
   },
 });
 
+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({
   __proto__: Metrics.Provider.prototype,
 
   name: "org.mozilla.crashes",
 
-  measurementTypes: [DailyCrashesMeasurement],
+  measurementTypes: [
+    DailyCrashesMeasurement1,
+    DailyCrashesMeasurement2,
+  ],
 
   pullOnly: true,
 
-  collectConstantData: function () {
+  collectDailyData: function () {
     return this.storage.enqueueTransaction(this._populateCrashCounts.bind(this));
   },
 
   _populateCrashCounts: function () {
-    let now = new Date();
-    let service = new CrashDirectoryService();
-
-    let pending = yield service.getPendingFiles();
-    let submitted = yield service.getSubmittedFiles();
-
-    function getAgeLimit() {
-      return 0;
-    }
-
-    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(),
+    this._log.info("Grabbing crash counts from crash manager.");
+    let crashCounts = yield this._manager.getCrashCountsByDay();
+    let fields = {
+      "main-crash": "mainCrash",
     };
 
-    // 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;
+    let m = this.getMeasurement("crashes", 2);
 
-      if (modified.getTime() < lastCheck) {
-        continue;
-      }
-
-      counts.submitted.appendValue(modified, 1);
-    }
+    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;
+        }
 
-    for (let [date, values] in counts.pending) {
-      yield m.incrementDailyCounter("pending", date, values.length);
+        yield m.setDailyLastNumeric(fields[type], count, date);
+      }
     }
-
-    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();
-      }
-    });
-  },
-});
+#endif
 
 
 /**
  * Holds basic statistics about the Places database.
  */
 function PlacesMeasurement() {
   Metrics.Measurement.call(this);
 }
--- a/services/healthreport/tests/xpcshell/test_provider_crashes.js
+++ b/services/healthreport/tests/xpcshell/test_provider_crashes.js
@@ -3,154 +3,86 @@
 
 "use strict";
 
 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/AppData.jsm");
-
-
-const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000;
+Cu.import("resource://testing-common/CrashManagerTest.jsm");
 
 
 function run_test() {
   run_next_test();
 }
 
-// run_test() needs to finish synchronously, so we do async init here.
-add_task(function test_init() {
+add_task(function* init() {
+  do_get_profile();
   yield makeFakeAppDir();
 });
 
-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_constructor() {
+  let provider = new CrashesProvider();
 });
 
-add_test(function test_constructor() {
-  let provider = new CrashesProvider();
-
-  run_next_test();
-});
-
-add_task(function test_init() {
+add_task(function* test_init() {
   let storage = yield Metrics.Storage("init");
   let provider = new CrashesProvider();
   yield provider.init(storage);
   yield provider.shutdown();
 
   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);
 
-  // FUTURE Don't rely on state from previous test.
-  yield provider.collectConstantData();
+  // Install custom manager so we don't interfere with other tests.
+  let manager = yield getManager();
+  provider._manager = manager;
+
+  let day1 = new Date(2014, 0, 1, 0, 0, 0);
+  let day2 = new Date(2014, 0, 3, 0, 0, 0);
 
-  let m = provider.getMeasurement("crashes", 1);
-  let values = yield m.getValues();
-  do_check_eq(values.days.size, Object.keys(gPending).length);
-  for each (let date in gPending) {
-    do_check_true(values.days.hasDay(date));
+  // 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 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);
-  }
+  let m = provider.getMeasurement("crashes", 2);
+  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));
+
+  let value = values.days.getDay(day1);
+  do_check_true(value.has("mainCrash"));
+  do_check_eq(value.get("mainCrash"), 2);
 
-  let currentState = yield provider.getState("lastCheck");
-  do_check_eq(typeof(currentState), "string");
-  do_check_true(currentState.length > 0);
-  let lastState = currentState;
+  value = values.days.getDay(day2);
+  do_check_eq(value.get("mainCrash"), 1);
 
-  // If we collect again, we should get no new data.
-  yield provider.collectConstantData();
+  // 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();
   values = yield m.getValues();
-  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);
-  }
+  value = values.days.getDay(day2);
+  do_check_eq(value.get("mainCrash"), 2);
 
   yield provider.shutdown();
   yield storage.close();
 });
 
--- a/services/healthreport/tests/xpcshell/xpcshell.ini
+++ b/services/healthreport/tests/xpcshell/xpcshell.ini
@@ -3,13 +3,14 @@ head = head.js
 tail =
 
 [test_load_modules.js]
 [test_profile.js]
 [test_healthreporter.js]
 [test_provider_addons.js]
 [test_provider_appinfo.js]
 [test_provider_crashes.js]
+run-if = crashreporter
 [test_provider_places.js]
 [test_provider_searches.js]
 [test_provider_sysinfo.js]
 [test_provider_sessions.js]
 
--- a/toolkit/components/crashes/CrashManager.jsm
+++ b/toolkit/components/crashes/CrashManager.jsm
@@ -511,16 +511,24 @@ this.CrashManager.prototype = Object.fre
    */
   getCrashes: function () {
     return Task.spawn(function* () {
       let store = yield this._getStore();
 
       return store.crashes;
     }.bind(this));
   },
+
+  getCrashCountsByDay: function () {
+    return Task.spawn(function* () {
+      let store = yield this._getStore();
+
+      return store._countsByDay;
+    }.bind(this));
+  },
 });
 
 let gCrashManager;
 
 /**
  * Interface to storage of crash data.
  *
  * This type handles storage of crash metadata. It exists as a separate type
--- a/toolkit/components/crashes/docs/index.rst
+++ b/toolkit/components/crashes/docs/index.rst
@@ -1,8 +1,10 @@
+.. _crashes_crashmanager:
+
 =============
 Crash Manager
 =============
 
 The **Crash Manager** is a service and interface for managing crash
 data within the Gecko application.
 
 From JavaScript, the service can be accessed via::