Bug 910563 - gracefully handle the thumbnail process crashing. r=adw

toolkit/components/thumbnails/BackgroundPageThumbs.jsm

@@ -69,6 +69,30 @@ const BackgroundPageThumbs = {
     this._processCaptureQueue();
   },
 
+  observe: function(subject, topic, data) {
+    // oop-frameloader-crashed - see if it was ours?
+    if (this._thumbBrowser) {
+      let frameLoader = subject.QueryInterface(Ci.nsIFrameLoader);
+      if (this._thumbBrowser.messageManager == frameLoader.messageManager) {
+        Cu.reportError("BackgroundThumbnails remote process crashed - recovering");
+        this._destroyBrowser();
+        let curCapture = this._captureQueue.length ? this._captureQueue[0] : null;
+        // we could retry the pending capture, but it's possible the crash
+        // was due directly to it, so trying again might just crash again.
+        // We could keep a flag to indicate if it previously crashed, but
+        // "resetting" the capture requires more work - so for now, we just
+        // discard it.
+        if (curCapture && curCapture.pending) {
+          curCapture._done(null);
+          // _done automatically continues queue processing.
+        }
+        // else: we must have been idle and not currently doing a capture (eg,
+        // maybe a GC or similar crashed) - so there's no need to attempt a
+        // queue restart - the next capture request will set everything up.
+      }
+    }
+  },
+
   /**
    * Ensures that initialization of the thumbnail browser's parent window has
    * begun.
@@ -149,11 +173,18 @@ const BackgroundPageThumbs = {
 
     browser.messageManager.loadFrameScript(FRAME_SCRIPT_URL, false);
     this._thumbBrowser = browser;
+    // an observer to notice if the remote process crashes.  There is also an
+    // "ipc:browser-destroyed" sent both for normal and abnormal terminations,
+    // but it's not currently possible to determine what browser it was for
+    // (although nsITabParent is probably going to grow a way of determining
+    // it at some point)
+    Services.obs.addObserver(this, "oop-frameloader-crashed", false);
   },
 
   _destroyBrowser: function () {
     if (!this._thumbBrowser)
       return;
+    Services.obs.removeObserver(this, "oop-frameloader-crashed");
     this._thumbBrowser.remove();
     delete this._thumbBrowser;
   },
@@ -178,7 +209,8 @@ const BackgroundPageThumbs = {
   },
 
   /**
-   * Called when the current capture completes or times out.
+   * Called when the current capture completes or fails (eg, times out, remote
+   * process crashes.)
    */
   _onCaptureOrTimeout: function (capture) {
     // Since timeouts start as an item is being processed, only the first

toolkit/components/thumbnails/test/Makefile.in

@@ -4,6 +4,7 @@
 
 MOCHITEST_BROWSER_FILES := \
 	browser_thumbnails_background.js \
+	browser_thumbnails_background_crash.js \
 	browser_thumbnails_capture.js \
 	browser_thumbnails_expiration.js \
 	browser_thumbnails_privacy.js \
@@ -20,5 +21,6 @@ MOCHITEST_BROWSER_FILES := \
 	background_red_redirect.sjs \
 	privacy_cache_control.sjs \
 	thumbnails_background.sjs \
+	thumbnails_crash_content_helper.js \
 	thumbnails_update.sjs \
 	$(NULL)

toolkit/components/thumbnails/test/browser_thumbnails_background_crash.js

@@ -0,0 +1,117 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const TEST_PAGE_URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_background.sjs";
+const TEST_CONTENT_HELPER = "chrome://mochitests/content/browser/toolkit/components/thumbnails/test/thumbnails_crash_content_helper.js";
+
+const imports = {};
+Cu.import("resource://gre/modules/BackgroundPageThumbs.jsm", imports);
+Cu.import("resource://gre/modules/PageThumbs.jsm", imports);
+Cu.import("resource://gre/modules/Task.jsm", imports);
+Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js", imports);
+
+function test() {
+  waitForExplicitFinish();
+
+  spawnNextTest();
+}
+
+function spawnNextTest() {
+  if (!tests.length) {
+    finish();
+    return;
+  }
+  let test = tests.shift();
+  info("Running sub-test " + test.name);
+  imports.Task.spawn(test).then(spawnNextTest, function onError(err) {
+    ok(false, err);
+    spawnNextTest();
+  });
+}
+
+let tests = [
+
+  function crashDuringCapture() {
+    // make a good capture first - this ensures we have the <browser>
+    let goodUrl = testPageURL();
+    yield capture(goodUrl);
+    let goodFile = fileForURL(goodUrl);
+    ok(goodFile.exists(), "Thumbnail should be cached after capture: " + goodFile.path);
+    goodFile.remove(false);
+    // inject our content script.
+    let mm = injectContentScript();
+    // queue up 2 captures - the first has a wait, so this is the one that
+    // will die.  The second one should immediately capture after the crash.
+    let waitUrl = testPageURL({ wait: 30000 });
+    let deferred1 = capture(waitUrl);
+    let deferred2 = capture(goodUrl);
+    info("Crashing the thumbnail content process.");
+    mm.sendAsyncMessage("thumbnails-test:crash");
+    yield deferred1;
+    let waitFile = fileForURL(waitUrl);
+    ok(!waitFile.exists(), "Thumbnail should not have been saved due to the crash: " + waitFile.path);
+    yield deferred2;
+    ok(goodFile.exists(), "We should have recovered and completed the 2nd capture after the crash: " + goodFile.path);
+    goodFile.remove(false);
+  },
+
+  function crashWhileIdle() {
+    // make a good capture first - this ensures we have the <browser>
+    let goodUrl = testPageURL();
+    yield capture(goodUrl);
+    let goodFile = fileForURL(goodUrl);
+    ok(goodFile.exists(), "Thumbnail should be cached after capture: " + goodFile.path);
+    goodFile.remove(false);
+    // inject our content script.
+    let mm = injectContentScript();
+    // the observer for the crashing process is basically async, so it's hard
+    // to know when the <browser> has actually seen it.  Easist is to just add
+    // our own observer.
+    let deferred = imports.Promise.defer();
+    Services.obs.addObserver(function crashObserver() {
+      Services.obs.removeObserver(crashObserver, "oop-frameloader-crashed");
+      // spin the event loop to ensure the BPT observer was called.
+      executeSoon(function() {
+        // Now queue another capture and ensure it recovers.
+        capture(goodUrl).then(() => {
+          ok(goodFile.exists(), "We should have recovered and handled new capture requests: " + goodFile.path);
+          goodFile.remove(false);
+          deferred.resolve();
+        });
+      });
+    } , "oop-frameloader-crashed", false);
+
+    // Nothing is pending - crash the process.
+    info("Crashing the thumbnail content process.");
+    mm.sendAsyncMessage("thumbnails-test:crash");
+    yield deferred.promise;
+  },
+];
+
+function injectContentScript() {
+  let thumbnailBrowser = imports.BackgroundPageThumbs._thumbBrowser;
+  let mm = thumbnailBrowser.messageManager;
+  mm.loadFrameScript(TEST_CONTENT_HELPER, false);
+  return mm;
+}
+
+function capture(url, options) {
+  let deferred = imports.Promise.defer();
+  options = options || {};
+  options.onDone = function onDone(capturedURL) {
+    deferred.resolve(capturedURL);
+  };
+  imports.BackgroundPageThumbs.capture(url, options);
+  return deferred.promise;
+}
+
+function fileForURL(url) {
+  let path = imports.PageThumbsStorage.getFilePathForURL(url);
+  let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
+  file.initWithPath(path);
+  return file;
+}
+
+function testPageURL(opts) {
+  return TEST_PAGE_URL + "?" + encodeURIComponent(JSON.stringify(opts || {}));
+}

toolkit/components/thumbnails/test/thumbnails_crash_content_helper.js

@@ -0,0 +1,29 @@
+/* Any copyright is dedicated to the Public Domain.
+* http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Ideally we would use CrashTestUtils.jsm, but that's only available for
+// xpcshell tests - so we just copy a ctypes crasher from it.
+Cu.import("resource://gre/modules/ctypes.jsm");
+let crash = function() { // this will crash when called.
+  let zero = new ctypes.intptr_t(8);
+  let badptr = ctypes.cast(zero, ctypes.PointerType(ctypes.int32_t));
+  badptr.contents
+};
+
+
+TestHelper = {
+  init: function() {
+    addMessageListener("thumbnails-test:crash", this);
+  },
+
+  receiveMessage: function(msg) {
+    switch (msg.name) {
+      case "thumbnails-test:crash":
+        privateNoteIntentionalCrash();
+        crash();
+      break;
+    }
+  },
+}
+
+TestHelper.init();