Bug 1399200 - Don't stop loading thumbnail when image redirects r=Mardak
authorahillier <ahillier@mozilla.com>
Tue, 12 Sep 2017 14:07:33 -0400
changeset 429954 ce559fa15078cd2c71ea0a16ae3191eac5bee177
parent 429953 1ca25d95e1aac287652580b29e3220201c1d6086
child 429955 f05ca536673199af9256016b6f7ed34296f2c897
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMardak
bugs1399200
milestone57.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 1399200 - Don't stop loading thumbnail when image redirects r=Mardak MozReview-Commit-ID: 1sADsZM6uYj
toolkit/components/thumbnails/BackgroundPageThumbs.jsm
toolkit/components/thumbnails/PageThumbUtils.jsm
toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
toolkit/components/thumbnails/test/browser_thumbnails_bg_image_capture.js
--- 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],