Bug 987101 - Switch the CrashMonitor to native OS.File.read(). r=Yoric

toolkit/components/crashmonitor/CrashMonitor.jsm

@@ -41,8 +41,6 @@ Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/AsyncShutdown.jsm");
-Cu.import("resource://gre/modules/NetUtil.jsm");
-Cu.import("resource://gre/modules/FileUtils.jsm");
 
 const NOTIFICATIONS = [
   "final-ui-startup",
@@ -100,42 +98,30 @@ let CrashMonitorInternal = {
    * @return {Promise} A promise that resolves/rejects once loading is complete
    */
   loadPreviousCheckpoints: function () {
-    let deferred = Promise.defer();
-    CrashMonitorInternal.previousCheckpoints = deferred.promise;
-
-    let file = FileUtils.File(CrashMonitorInternal.path);
-    NetUtil.asyncFetch(file, function(inputStream, status) {
-      if (!Components.isSuccessCode(status)) {
-        if (status != Cr.NS_ERROR_FILE_NOT_FOUND) {
-          Cu.reportError("Error while loading crash monitor data: " + status);
+    this.previousCheckpoints = Task.spawn(function*() {
+      let data;
+      try {
+        data = yield OS.File.read(CrashMonitorInternal.path, { encoding: "utf-8" });
+      } catch (ex if ex instanceof OS.File.Error) {
+        if (!ex.becauseNoSuchFile) {
+          Cu.reportError("Error while loading crash monitor data: " + ex.toString());
         }
 
-        deferred.resolve(null);
-        return;
+        return null;
       }
 
-      let data = NetUtil.readInputStreamToString(inputStream,
-        inputStream.available(), { charset: "UTF-8" });
-
-      let notifications = null;
+      let notifications;
       try {
         notifications = JSON.parse(data);
       } catch (ex) {
         Cu.reportError("Error while parsing crash monitor data: " + ex);
-        deferred.resolve(null);
+        return null;
       }
 
-      try {
-        deferred.resolve(Object.freeze(notifications));
-      } catch (ex) {
-        // The only exception we reject from is if notifications is not
-        // an object. This happens when the checkpoints file contained
-        // just a numeric string.
-        deferred.reject(ex);
-      }
+      return Object.freeze(notifications);
     });
 
-    return deferred.promise;
+    return this.previousCheckpoints;
   }
 };
 

toolkit/components/crashmonitor/test/unit/test_invalid_json.js

@@ -0,0 +1,18 @@
+/* -*- Mode: js; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* 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/. */
+
+/**
+ * Test with sessionCheckpoints.json containing invalid JSON data
+ */
+add_task(function test_invalid_file() {
+  // Write bogus data to checkpoint file
+  let data = "[}";
+  yield OS.File.writeAtomic(sessionCheckpointsPath, data,
+                            {tmpPath: sessionCheckpointsPath + ".tmp"});
+
+  CrashMonitor.init();
+  let checkpoints = yield CrashMonitor.previousCheckpoints;
+  do_check_eq(checkpoints, null);
+});

toolkit/components/crashmonitor/test/unit/xpcshell.ini

@@ -5,5 +5,6 @@ tail =
 [test_init.js]
 [test_valid_file.js]
 [test_invalid_file.js]
+[test_invalid_json.js]
 [test_missing_file.js]
 [test_register.js]