Bug 910563 - gracefully handle the thumbnail process crashing. r=adw
☠☠ backed out by a66653f84ac7 ☠ ☠
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 10 Sep 2013 12:16:26 +1000
changeset 146326 b8a5df4364044fc83529005d87e798b6c9e0392e
parent 146325 dedfb60795aab1b72cf6264e2d1bd3f1011fba70
child 146327 87f7da1e96632d6099c127bf5859417dd6a59865
push id25252
push usercbook@mozilla.com
push dateTue, 10 Sep 2013 08:27:30 +0000
treeherdermozilla-central@9f9733d4c20e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs910563
milestone26.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 910563 - gracefully handle the thumbnail process crashing. r=adw
toolkit/components/thumbnails/BackgroundPageThumbs.jsm
toolkit/components/thumbnails/test/Makefile.in
toolkit/components/thumbnails/test/browser_thumbnails_background_crash.js
toolkit/components/thumbnails/test/thumbnails_crash_content_helper.js
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -64,16 +64,40 @@ const BackgroundPageThumbs = {
       return;
     }
     let cap = new Capture(url, this._onCaptureOrTimeout.bind(this), options);
     this._captureQueue.push(cap);
     this._capturesByURL.set(url, cap);
     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.
    *
    * @return  True if the parent window is completely initialized and can be
    *          used, and false if initialization has started but not completed.
    */
   _ensureParentWindowReady: function () {
@@ -144,21 +168,28 @@ const BackgroundPageThumbs = {
     // thumbnails are blank and transparent -- but setting the style does.
     browser.style.width = bwidth + "px";
     browser.style.height = (bwidth * sheight.value / swidth.value) + "px";
 
     this._parentWin.document.documentElement.appendChild(browser);
 
     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;
   },
 
   /**
    * Starts the next capture if the queue is not empty and the service is fully
    * initialized.
    */
@@ -173,17 +204,18 @@ const BackgroundPageThumbs = {
     this._captureQueue[0].start(this._thumbBrowser.messageManager);
     if (this._destroyBrowserTimer) {
       this._destroyBrowserTimer.cancel();
       delete this._destroyBrowserTimer;
     }
   },
 
   /**
-   * 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
     // item in the queue can be passed to this method.
     if (capture !== this._captureQueue[0])
       throw new Error("The capture should be at the head of the queue.");
     this._captureQueue.shift();
     this._capturesByURL.delete(capture.url);
--- a/toolkit/components/thumbnails/test/Makefile.in
+++ b/toolkit/components/thumbnails/test/Makefile.in
@@ -1,24 +1,26 @@
 # 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/.
 
 MOCHITEST_BROWSER_FILES := \
 	browser_thumbnails_background.js \
+	browser_thumbnails_background_crash.js \
 	browser_thumbnails_capture.js \
 	browser_thumbnails_expiration.js \
 	browser_thumbnails_privacy.js \
 	browser_thumbnails_redirect.js \
 	browser_thumbnails_storage.js \
 	browser_thumbnails_storage_migrate3.js \
 	browser_thumbnails_bug726727.js \
 	browser_thumbnails_bug727765.js \
 	browser_thumbnails_bug818225.js \
 	browser_thumbnails_update.js \
 	head.js \
 	background_red.html \
 	background_red_scroll.html \
 	background_red_redirect.sjs \
 	privacy_cache_control.sjs \
 	thumbnails_background.sjs \
+	thumbnails_crash_content_helper.js \
 	thumbnails_update.sjs \
 	$(NULL)
new file mode 100644
--- /dev/null
+++ b/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 || {}));
+}
new file mode 100644
--- /dev/null
+++ b/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();