Bug 1399200 - Don't stop loading thumbnail when image redirects
MozReview-Commit-ID: 1sADsZM6uYj
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -24,22 +24,26 @@ Cu.import("resource://gre/modules/PageTh
Cu.import("resource://gre/modules/Services.jsm");
// possible FX_THUMBNAILS_BG_CAPTURE_DONE_REASON_2 telemetry values
const TEL_CAPTURE_DONE_OK = 0;
const TEL_CAPTURE_DONE_TIMEOUT = 1;
// 2 and 3 were used when we had special handling for private-browsing.
const TEL_CAPTURE_DONE_CRASHED = 4;
const TEL_CAPTURE_DONE_BAD_URI = 5;
+const TEL_CAPTURE_DONE_LOAD_FAILED = 6;
+const TEL_CAPTURE_DONE_IMAGE_ZERO_DIMENSION = 7;
// These are looked up on the global as properties below.
XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_OK", TEL_CAPTURE_DONE_OK);
XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_TIMEOUT", TEL_CAPTURE_DONE_TIMEOUT);
XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_CRASHED", TEL_CAPTURE_DONE_CRASHED);
XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_BAD_URI", TEL_CAPTURE_DONE_BAD_URI);
+XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_LOAD_FAILED", TEL_CAPTURE_DONE_LOAD_FAILED);
+XPCOMUtils.defineConstant(this, "TEL_CAPTURE_DONE_IMAGE_ZERO_DIMENSION", TEL_CAPTURE_DONE_IMAGE_ZERO_DIMENSION);
XPCOMUtils.defineLazyModuleGetter(this, "ContextualIdentityService",
"resource://gre/modules/ContextualIdentityService.jsm");
const global = this;
const BackgroundPageThumbs = {
/**
@@ -55,16 +59,18 @@ const BackgroundPageThumbs = {
* onDone(url),
* where `url` is the captured URL.
* @opt timeout The capture will time out after this many milliseconds have
* elapsed after the capture has progressed to the head of
* the queue and started. Defaults to 30000 (30 seconds).
* @opt isImage If true, backgroundPageThumbsContent will attempt to render
* the url directly to canvas. Note that images will mostly get
* detected and rendered as such anyway, but this will ensure it.
+ * @opt targetWidth The target width when capturing an image.
+ * @opt backgroundColor The background colour when capturing an image.
*/
capture(url, options = {}) {
if (!PageThumbs._prefEnabled()) {
if (options.onDone)
schedule(() => options.onDone(url));
return;
}
this._captureQueue = this._captureQueue || [];
@@ -401,18 +407,23 @@ Capture.prototype = {
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);
// didCapture registration
this._msgMan = messageManager;
- this._msgMan.sendAsyncMessage("BackgroundPageThumbs:capture",
- { id: this.id, url: this.url, isImage: this.options.isImage });
+ this._msgMan.sendAsyncMessage("BackgroundPageThumbs:capture", {
+ id: this.id,
+ url: this.url,
+ isImage: this.options.isImage,
+ targetWidth: this.options.targetWidth,
+ backgroundColor: this.options.backgroundColor
+ });
this._msgMan.addMessageListener("BackgroundPageThumbs:didCapture", this);
},
/**
* 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.
*/
@@ -475,17 +486,17 @@ Capture.prototype = {
tel(id, data.telemetry[id]);
}
}
let done = () => {
captureCallback(this);
for (let callback of doneCallbacks) {
try {
- callback.call(options, this.url);
+ callback.call(options, this.url, this.doneReason);
} catch (err) {
Cu.reportError(err);
}
}
if (Services.prefs.getBoolPref(ABOUT_NEWTAB_SEGREGATION_PREF)) {
// Clear the data in the private container for thumbnails.
let privateIdentity =
--- a/toolkit/components/thumbnails/PageThumbUtils.jsm
+++ b/toolkit/components/thumbnails/PageThumbUtils.jsm
@@ -114,49 +114,51 @@ this.PageThumbUtils = {
return [width, height];
},
/**
* Renders an image onto a new canvas of a given width and proportional
* height. Uses an image that exists in the window and is loaded, or falls
* back to loading the url into a new image element.
*/
- async createImageThumbnailCanvas(window, url, targetWidth = 448) {
+ async createImageThumbnailCanvas(window, url, targetWidth = 448, backgroundColor = this.THUMBNAIL_BG_COLOR) {
// 224px is the width of cards in ActivityStream; capture thumbnails at 2x
const doc = (window || Services.appShell.hiddenDOMWindow).document;
let image = doc.querySelector("img");
- if (!image || image.src !== url) {
+ if (!image) {
image = doc.createElementNS(this.HTML_NAMESPACE, "img");
- }
- if (!image.complete) {
- await new Promise(resolve => {
+ await new Promise((resolve, reject) => {
image.onload = () => resolve();
- image.onerror = () => { throw new Error("Image failed to load"); }
+ image.onerror = () => reject(new Error("LOAD_FAILED"));
image.src = url;
});
}
// <img src="*.svg"> has width/height but not naturalWidth/naturalHeight
const imageWidth = image.naturalWidth || image.width;
const imageHeight = image.naturalHeight || image.height;
if (imageWidth === 0 || imageHeight === 0) {
- throw new Error("Image has zero dimension");
+ throw new Error("IMAGE_ZERO_DIMENSION");
}
const width = Math.min(targetWidth, imageWidth);
const height = imageHeight * width / imageWidth;
// As we're setting the width and maintaining the aspect ratio, if an image
// is very tall we might get a very large thumbnail. Restricting the canvas
// size to {width}x{width} solves this problem. Here we choose to clip the
// image at the bottom rather than centre it vertically, based on an
// estimate that the focus of a tall image is most likely to be near the top
// (e.g., the face of a person).
- const canvas = this.createCanvas(window, width, Math.min(height, width));
- canvas.getContext("2d").drawImage(image, 0, 0, width, height);
+ const canvasHeight = Math.min(height, width);
+ const canvas = this.createCanvas(window, width, canvasHeight);
+ const context = canvas.getContext("2d");
+ context.fillStyle = backgroundColor;
+ context.fillRect(0, 0, width, canvasHeight);
+ context.drawImage(image, 0, 0, width, height);
return canvas;
},
/** *
* Given a browser window, this creates a snapshot of the content
* and returns a canvas with the resulting snapshot of the content
* at the thumbnail size. It has to do this through a two step process:
*
--- a/toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
+++ b/toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
@@ -73,17 +73,19 @@ const backgroundPageThumbsContent = {
get _webNav() {
return docShell.QueryInterface(Ci.nsIWebNavigation);
},
_onCapture(msg) {
this._nextCapture = {
id: msg.data.id,
url: msg.data.url,
- isImage: msg.data.isImage
+ isImage: msg.data.isImage,
+ targetWidth: msg.data.targetWidth,
+ backgroundColor: msg.data.backgroundColor
};
if (this._currentCapture) {
if (this._state == STATE_LOADING) {
// Cancel the current capture.
this._state = STATE_CANCELED;
this._loadAboutBlank();
}
// Let the current capture finish capturing, or if it was just canceled,
@@ -102,18 +104,16 @@ const backgroundPageThumbsContent = {
this._currentCapture.pageLoadStartDate = new Date();
try {
this._webNav.loadURI(this._currentCapture.url,
Ci.nsIWebNavigation.LOAD_FLAGS_STOP_CONTENT,
null, null, null);
} catch (e) {
this._failCurrentCapture("BAD_URI");
- delete this._currentCapture;
- this._startNextCapture();
}
},
onStateChange(webProgress, req, flags, status) {
if (webProgress.isTopLevel &&
(flags & Ci.nsIWebProgressListener.STATE_STOP) &&
this._currentCapture) {
if (req.name == "about:blank") {
@@ -157,43 +157,44 @@ const backgroundPageThumbsContent = {
delete this._cancelTimer;
}, 0, Ci.nsITimer.TYPE_ONE_SHOT);
}
}
}
},
_captureCurrentPage() {
- let win = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
- .getInterface(Ci.nsIDOMWindow);
- win.requestIdleCallback(async () => {
+ const doCapture = async () => {
let capture = this._currentCapture;
capture.finalURL = this._webNav.currentURI.spec;
capture.pageLoadTime = new Date() - capture.pageLoadStartDate;
let canvasDrawDate = new Date();
docShell.isActive = true;
let finalCanvas;
if (capture.isImage || content.document instanceof content.ImageDocument) {
- finalCanvas = await PageThumbUtils.createImageThumbnailCanvas(content, capture.url);
+ finalCanvas = await PageThumbUtils.createImageThumbnailCanvas(content, capture.url, capture.targetWidth, capture.backgroundColor);
} else {
finalCanvas = PageThumbUtils.createSnapshotThumbnail(content, null);
}
docShell.isActive = false;
capture.canvasDrawTime = new Date() - canvasDrawDate;
finalCanvas.toBlob(blob => {
capture.imageBlob = new Blob([blob]);
// Load about:blank to finish the capture and wait for onStateChange.
this._loadAboutBlank();
});
- });
+ };
+ let win = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+ .getInterface(Ci.nsIDOMWindow);
+ win.requestIdleCallback(() => doCapture().catch(ex => this._failCurrentCapture(ex.message)));
},
_finishCurrentCapture() {
let capture = this._currentCapture;
let fileReader = new FileReader();
fileReader.onloadend = () => {
sendAsyncMessage("BackgroundPageThumbs:didCapture", {
id: capture.id,
@@ -209,16 +210,18 @@ const backgroundPageThumbsContent = {
},
_failCurrentCapture(reason) {
let capture = this._currentCapture;
sendAsyncMessage("BackgroundPageThumbs:didCapture", {
id: capture.id,
failReason: reason,
});
+ delete this._currentCapture;
+ this._startNextCapture();
},
// We load about:blank to finish all captures, even canceled captures. Two
// reasons: GC the captured page, and ensure it can't possibly load any more
// resources.
_loadAboutBlank: function _loadAboutBlank() {
// It's possible we've been destroyed by now, if so don't do anything:
if (!docShell) {
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_image_capture.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_image_capture.js
@@ -4,16 +4,25 @@
const BASE_URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/";
/**
* These tests ensure that when trying to capture a url that is an image file,
* the image itself is captured instead of the the browser window displaying the
* image, and that the thumbnail maintains the image aspect ratio.
*/
function* runTests() {
+ // Test that malformed input causes _finishCurrentCapture to be called with
+ // the correct reason.
+ const emptyUrl = "data:text/plain,";
+ yield bgCapture(emptyUrl, {isImage: true, onDone: (url, reason) => {
+ // BackgroundPageThumbs.TEL_CAPTURE_DONE_LOAD_FAILED === 6
+ is(reason, 6, "Should have the right failure reason");
+ next();
+ }});
+
for (const {url, color, width, height} of [{
url: BASE_URL + "test/sample_image_red_1920x1080.jpg",
color: [255, 0, 0],
width: 1920,
height: 1080
}, {
url: BASE_URL + "test/sample_image_green_1024x1024.jpg",
color: [0, 255, 0],