Bug 890130 - Background thumbnail timeouts now start when item starts processing. r=adw

This commit is contained in:
Mark Hammond 2013-07-12 13:59:16 +10:00
parent 8dd45769d2
commit 499b4ae55c
2 changed files with 30 additions and 67 deletions

View File

@ -34,8 +34,8 @@ const BackgroundPageThumbs = {
* onDone(url),
* where `url` is the captured URL.
* @opt timeout The capture will time out after this many milliseconds have
* elapsed after calling this method. Defaults to 30000 (30
* seconds).
* elapsed after the capture has progressed to the head of
* the queue and started. Defaults to 30000 (30 seconds).
*/
capture: function (url, options={}) {
if (isPrivateBrowsingActive()) {
@ -186,16 +186,14 @@ const BackgroundPageThumbs = {
},
/**
* Called when a capture completes or times out.
* Called when the current capture completes or times out.
*/
_onCaptureOrTimeout: function (capture) {
// Since timeouts are configurable per capture, and a capture's timeout
// timer starts when it's created, it's possible for any capture to time
// out regardless of its position in the queue.
let idx = this._captureQueue.indexOf(capture);
if (idx < 0)
throw new Error("The capture should be in the queue.");
this._captureQueue.splice(idx, 1);
// 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);
// Start the destroy-browser timer *before* processing the capture queue.
@ -227,13 +225,6 @@ function Capture(url, captureCallback, options) {
this.doneCallbacks = [];
if (options.onDone)
this.doneCallbacks.push(options.onDone);
// The timeout starts when the consumer requests the capture, not when the
// capture is dequeued and started.
let timeout = typeof(options.timeout) == "number" ? options.timeout :
DEFAULT_CAPTURE_TIMEOUT;
this._timeoutTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
this._timeoutTimer.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
}
Capture.prototype = {
@ -248,6 +239,11 @@ Capture.prototype = {
* @param messageManager The nsIMessageSender of the thumbnail browser.
*/
start: function (messageManager) {
let timeout = typeof(this.options.timeout) == "number" ? this.options.timeout :
DEFAULT_CAPTURE_TIMEOUT;
this._timeoutTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
this._timeoutTimer.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
this._msgMan = messageManager;
this._msgMan.sendAsyncMessage("BackgroundPageThumbs:capture",
{ id: this.id, url: this.url });
@ -258,15 +254,15 @@ Capture.prototype = {
* The only intended external use of this method is by the service when it's
* uninitializing and doing things like destroying the thumbnail browser. In
* that case the consumer's completion callback will never be called.
*
* This method is not idempotent. It's an error to call it more than once on
* the same capture.
*/
destroy: function () {
this._timeoutTimer.cancel();
delete this._timeoutTimer;
// This method may be called for captures that haven't started yet, so
// guard against not yet having _timeoutTimer, _msgMan etc properties...
if (this._timeoutTimer) {
this._timeoutTimer.cancel();
delete this._timeoutTimer;
}
if (this._msgMan) {
// The capture may have never started, so _msgMan may be undefined.
this._msgMan.removeMessageListener("BackgroundPageThumbs:didCapture",
this);
delete this._msgMan;

View File

@ -47,20 +47,29 @@ let tests = [
let urls = [
"http://www.example.com/0",
"http://www.example.com/1",
// an item that will timeout to ensure timeouts work and we resume.
testPageURL({ wait: 2002 }),
"http://www.example.com/2",
];
let files = urls.map(fileForURL);
files.forEach(f => ok(!f.exists(), "Thumbnail should not be cached yet."));
urls.forEach(function (url) {
let isTimeoutTest = url.indexOf("?wait") >= 0;
imports.BackgroundPageThumbs.capture(url, {
timeout: isTimeoutTest ? 100 : 30000,
onDone: function onDone(capturedURL) {
ok(urls.length > 0, "onDone called, so URLs should still remain");
is(capturedURL, urls.shift(),
"Captured URL should be currently expected URL (i.e., " +
"capture() callbacks should be called in the correct order)");
let file = files.shift();
ok(file.exists(),
"Thumbnail should be cached after capture: " + file.path);
if (isTimeoutTest) {
ok(!file.exists(),
"Thumbnail shouldn't exist for timed out capture: " + file.path);
} else {
ok(file.exists(),
"Thumbnail should be cached after capture: " + file.path);
}
if (!urls.length)
deferred.resolve();
},
@ -88,48 +97,6 @@ let tests = [
yield deferred.promise;
},
function timeoutQueueing() {
let deferred = imports.Promise.defer();
let urls = [
{ url: testPageURL({ wait: 2000 }), timeout: 30000 },
{ url: testPageURL({ wait: 2001 }), timeout: 1000 },
{ url: testPageURL({ wait: 2002 }), timeout: 0 },
];
// The expected callback order is the reverse of the above, and the reverse
// of the order in which the captures are made.
let expectedOrder = urls.slice();
expectedOrder.reverse();
expectedOrder = expectedOrder.map(u => u.url);
let files = expectedOrder.map(fileForURL);
files.forEach(f => ok(!f.exists(), "Thumbnail should not be cached yet."));
urls.forEach(function ({ url, timeout }) {
imports.BackgroundPageThumbs.capture(url, {
timeout: timeout,
onDone: function onDone(capturedURL) {
ok(expectedOrder.length > 0,
"onDone called, so URLs should still remain");
is(capturedURL, expectedOrder.shift(),
"Captured URL should be currently expected URL (i.e., " +
"capture() callbacks should be called in the correct order)");
let file = files.shift();
if (timeout > 2000)
ok(file.exists(),
"Thumbnail should be cached after capture: " + file.path);
else
ok(!file.exists(),
"Capture timed out so thumbnail should not be cached: " +
file.path);
if (!expectedOrder.length)
deferred.resolve();
},
});
});
yield deferred.promise;
},
function redirect() {
let finalURL = "http://example.com/redirected";
let originalURL = testPageURL({ redirect: finalURL });