☠☠ backed out by a66653f84ac7 ☠ ☠ | |
author | Mark Hammond <mhammond@skippinet.com.au> |
Tue, 10 Sep 2013 12:16:26 +1000 | |
changeset 146326 | b8a5df4364044fc83529005d87e798b6c9e0392e |
parent 146325 | dedfb60795aab1b72cf6264e2d1bd3f1011fba70 |
child 146327 | 87f7da1e96632d6099c127bf5859417dd6a59865 |
push id | 25252 |
push user | cbook@mozilla.com |
push date | Tue, 10 Sep 2013 08:27:30 +0000 |
treeherder | mozilla-central@9f9733d4c20e [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | adw |
bugs | 910563 |
milestone | 26.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
|
--- 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();