Backed out changeset 67714091ce5b (bug 897880) for suspecion of causing a orange mochitest failure

toolkit/components/thumbnails/BackgroundPageThumbs.jsm

@@ -354,8 +354,7 @@ Capture.prototype = {
       callOnDones();
       return;
     }
-    PageThumbs._store(this.url, data.finalURL, data.imageData, data.wasErrorResponse)
-              .then(callOnDones);
+    PageThumbs._store(this.url, data.finalURL, data.imageData).then(callOnDones);
   },
 };
 

toolkit/components/thumbnails/PageThumbs.jsm

@@ -203,24 +203,18 @@ this.PageThumbs = {
    * capture process will send a notification via the observer service on
    * capture, so consumers should watch for such observations if they want to
    * be notified of an updated thumbnail.
-   *
-   * @return {Promise} that's resolved on completion.
    */
   captureIfStale: function PageThumbs_captureIfStale(aUrl) {
-    let deferredResult = Promise.defer();
     let filePath = PageThumbsStorage.getFilePathForURL(aUrl);
-    PageThumbsWorker.post(
-      "isFileRecent",
-      [filePath, MAX_THUMBNAIL_AGE_SECS]
+    PageThumbsWorker.post("isFileRecent", [filePath, MAX_THUMBNAIL_AGE_SECS]
     ).then(result => {
       if (!result.ok) {
         // Sadly there is currently a circular dependency between this module
         // and BackgroundPageThumbs, so do the import locally.
         let BPT = Cu.import("resource://gre/modules/BackgroundPageThumbs.jsm", {}).BackgroundPageThumbs;
-        BPT.capture(aUrl, {onDone: deferredResult.resolve});
+        BPT.capture(aUrl);
       }
     });
-    return deferredResult.promise;
   },
 
   /**
@@ -321,15 +315,12 @@ this.PageThumbs = {
     let channel = aBrowser.docShell.currentDocumentChannel;
     let originalURL = channel.originalURI.spec;
 
-    // see if this was an error response.
-    let wasError = this._isChannelErrorResponse(channel);
-
     TaskUtils.spawn((function task() {
       let isSuccess = true;
       try {
         let blob = yield this.captureToBlob(aBrowser.contentWindow);
         let buffer = yield TaskUtils.readBlob(blob);
-        yield this._store(originalURL, url, buffer, wasError);
+        yield this._store(originalURL, url, buffer);
       } catch (_) {
         isSuccess = false;
       }
@@ -347,27 +338,10 @@ this.PageThumbs = {
    * @param aOriginalURL The URL with which the capture was initiated.
    * @param aFinalURL The URL to which aOriginalURL ultimately resolved.
    * @param aData An ArrayBuffer containing the image data.
-   * @param aWasErrorResponse A boolean indicating if the capture was for a
-   *                          response that returned an error code.
    * @return {Promise}
    */
-  _store: function PageThumbs__store(aOriginalURL, aFinalURL, aData, aWasErrorResponse) {
+  _store: function PageThumbs__store(aOriginalURL, aFinalURL, aData) {
     return TaskUtils.spawn(function () {
-      // If we got an error response, we only save it if we don't have an
-      // existing thumbnail.  If we *do* have an existing thumbnail we "touch"
-      // it so we consider the old version fresh.
-      if (aWasErrorResponse) {
-        let result = yield PageThumbsStorage.touchIfExists(aFinalURL);
-        let exists = result.ok;
-        if (exists) {
-          if (aFinalURL != aOriginalURL) {
-             yield PageThumbsStorage.touchIfExists(aOriginalURL);
-          }
-          return;
-        }
-        // was an error response, but no existing thumbnail - just store
-        // that error response as something is (arguably) better than nothing.
-      }
       let telemetryStoreTime = new Date();
       yield PageThumbsStorage.writeData(aFinalURL, aData);
       Services.telemetry.getHistogramById("FX_THUMBNAILS_STORE_TIME_MS")
@@ -489,25 +463,6 @@ this.PageThumbs = {
     return [this._thumbnailWidth, this._thumbnailHeight];
   },
 
-  /**
-   * Given a channel, returns true if it should be considered an "error
-   * response", false otherwise.
-   */
-  _isChannelErrorResponse: function(channel) {
-    // No valid document channel sounds like an error to me!
-    if (!channel)
-      return true;
-    if (!(channel instanceof Ci.nsIHttpChannel))
-      // it might be FTP etc, so assume it's ok.
-      return false;
-    try {
-      return !channel.requestSucceeded;
-    } catch (_) {
-      // not being able to determine success is surely failure!
-      return true;
-    }
-  },
-
   _prefEnabled: function PageThumbs_prefEnabled() {
     try {
       return !Services.prefs.getBoolPref("browser.pagethumbnails.capturing_disabled");
@@ -615,17 +570,6 @@ this.PageThumbsStorage = {
     return PageThumbsWorker.post("wipe", [this.path]);
   },
 
-  /**
-   * If the file holding the thumbnail for the given URL exists, update the
-   * modification time of the file to now and return true, otherwise return
-   * false.
-   *
-   * @return {Promise}
-   */
-  touchIfExists: function Storage_touchIfExists(aURL) {
-    return PageThumbsWorker.post("touchIfExists", [this.getFilePathForURL(aURL)]);
-  },
-
   _calculateMD5Hash: function Storage_calculateMD5Hash(aValue) {
     let hash = gCryptoHash;
     let value = gUnicodeConverter.convertToByteArray(aValue);

toolkit/components/thumbnails/PageThumbsWorker.js

@@ -176,28 +176,6 @@ let Agent = {
     } finally {
       iterator.close();
     }
-  },
-
-  touchIfExists: function Agent_touchIfExists(path) {
-    // No OS.File way to update the modification date of the file (bug 905509)
-    // so we open it for reading and writing, read 1 byte from the start of
-    // the file then write that byte back out.
-    // (Sadly it's impossible to use nsIFile here as we have no access to
-    // |Components|)
-    if (!File.exists(path)) {
-      return false;
-    }
-    let file = OS.File.open(path, { read: true, write: true });
-    try {
-      file.setPosition(0); // docs aren't clear on initial position, so seek to 0.
-      let byte = file.read(1);
-      file.setPosition(0);
-      file.write(byte);
-    } finally {
-      file.close();
-    }
-    return true;
-  },
-
+   }
 };
 

toolkit/components/thumbnails/content/backgroundPageThumbsContent.js

@@ -52,8 +52,6 @@ const backgroundPageThumbsContent = {
       PageThumbs._captureToCanvas(content, canvas);
       let captureTime = new Date() - captureDate;
 
-      let channel = docShell.currentDocumentChannel;
-      let isErrorResponse = PageThumbs._isChannelErrorResponse(channel);
       let finalURL = this._webNav.currentURI.spec;
       let fileReader = Cc["@mozilla.org/files/filereader;1"].
                        createInstance(Ci.nsIDOMFileReader);
@@ -66,7 +64,6 @@ const backgroundPageThumbsContent = {
             CAPTURE_PAGE_LOAD_TIME_MS: pageLoadTime,
             CAPTURE_CANVAS_DRAW_TIME_MS: captureTime,
           },
-          wasErrorResponse: isErrorResponse,
         });
       };
       canvas.toBlob(blob => fileReader.readAsArrayBuffer(blob));

toolkit/components/thumbnails/test/Makefile.in

@@ -28,7 +28,6 @@ MOCHITEST_BROWSER_FILES := \
 	background_red_redirect.sjs \
 	privacy_cache_control.sjs \
 	thumbnails_background.sjs \
-	thumbnails_update.sjs \
 	$(NULL)
 
 include $(topsrcdir)/config/rules.mk

toolkit/components/thumbnails/test/browser_thumbnails_update.js

@@ -5,56 +5,20 @@
  * These tests check the auto-update facility of the thumbnail service.
  */
 
+let numNotifications = 0;
+const URL = "data:text/html;charset=utf-8,<body%20bgcolor=ff0000></body>";
+
+function observe(subject, topic, data) {
+  is(topic, "page-thumbnail:create", "got expected topic");
+  is(data, URL, "data is our test URL");
+  if (++numNotifications == 2) {
+    // This is the final notification and signals test success...
+    Services.obs.removeObserver(observe, "page-thumbnail:create");
+    next();
+  }
+}
+
 function runTests() {
-  // A "trampoline" - a generator that iterates over sub-iterators
-  let tests = [
-    simpleCaptureTest,
-    errorResponseUpdateTest,
-    goodResponseUpdateTest,
-    foregroundErrorResponseUpdateTest,
-    foregroundGoodResponseUpdateTest
-  ];
-  for (let test of tests) {
-    info("Running subtest " + test.name);
-    for (let iterator of test())
-      yield iterator;
-  }
-}
-
-function ensureThumbnailStale(url) {
-  // We go behind the back of the thumbnail service and change the
-  // mtime of the file to be in the past.
-  let fname = PageThumbsStorage.getFilePathForURL(url);
-  let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
-  file.initWithPath(fname);
-  ok(file.exists(), fname + " should exist");
-  // Set it as very stale...
-  file.lastModifiedTime = Date.now() - 1000000000;
-}
-
-function getThumbnailModifiedTime(url) {
-  let fname = PageThumbsStorage.getFilePathForURL(url);
-  let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
-  file.initWithPath(fname);
-  return file.lastModifiedTime;
-}
-
-// The tests!
-/* Check functionality of a normal "captureIfStale" request */
-function simpleCaptureTest() {
-  let numNotifications = 0;
-  const URL = "data:text/html;charset=utf-8,<body%20bgcolor=ff0000></body>";
-
-  function observe(subject, topic, data) {
-    is(topic, "page-thumbnail:create", "got expected topic");
-    is(data, URL, "data is our test URL");
-    if (++numNotifications == 2) {
-      // This is the final notification and signals test success...
-      Services.obs.removeObserver(observe, "page-thumbnail:create");
-      next();
-    }
-  }
-
   Services.obs.addObserver(observe, "page-thumbnail:create", false);
   // Create a tab with a red background.
   yield addTab(URL);
@@ -62,8 +26,6 @@ function simpleCaptureTest() {
 
   // Capture the screenshot.
   PageThumbs.captureAndStore(browser, function () {
-    // done with the tab.
-    gBrowser.removeTab(gBrowser.selectedTab);
     // We've got a capture so should have seen the observer.
     is(numNotifications, 1, "got notification of item being created.");
     // The capture is now "fresh" - so requesting the URL should not cause
@@ -71,89 +33,18 @@ function simpleCaptureTest() {
     PageThumbs.captureIfStale(URL);
     is(numNotifications, 1, "still only 1 notification of item being created.");
 
-    ensureThumbnailStale(URL);
+    // Now we will go behind the back of the thumbnail service and change the
+    // mtime of the file to be in the past.
+    let fname = PageThumbsStorage.getFilePathForURL(URL);
+    let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
+    file.initWithPath(fname);
+    ok(file.exists(), fname + " doesn't exist");
+    // Set it as very stale...
+    file.lastModifiedTime = Date.now() - 1000000000;
     // Ask for it to be updated.
     PageThumbs.captureIfStale(URL);
     // But it's async, so wait - our observer above will call next() when
     // the notification comes.
   });
-  yield undefined // wait for callbacks to call 'next'...
-}
-
-/* Check functionality of a background capture when there is an error response
-   from the server.
- */
-function errorResponseUpdateTest() {
-  const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?fail";
-  yield addTab(URL);
-
-  yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
-  gBrowser.removeTab(gBrowser.selectedTab);
-  // update the thumbnail to be stale, then re-request it.  The server will
-  // return a 400 response and a red thumbnail.
-  // The b/g service should (a) not save the thumbnail and (b) update the file
-  // to have an mtime of "now" - so we (a) check the thumbnail remains green
-  // and (b) check the mtime of the file is >= now.
-  ensureThumbnailStale(URL);
-  let now = Date.now();
-  PageThumbs.captureIfStale(URL).then(() => {
-    ok(getThumbnailModifiedTime(URL) >= now, "modified time should be >= now");
-    retrieveImageDataForURL(URL, function ([r, g, b]) {
-      is("" + [r,g,b], "" + [0, 255, 0], "thumbnail is still green");
-      next();
-    });
-  }).then(null, err => {ok(false, "Error in captureIfStale: " + err)});
-  yield undefined; // wait for callback to call 'next'...
-}
-
-/* Check functionality of a background capture when there is a non-error
-   response from the server.  This test is somewhat redundant - although it is
-   using a http:// URL instead of a data: url like most others.
- */
-function goodResponseUpdateTest() {
-  const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?ok";
-  yield addTab(URL);
-  let browser = gBrowser.selectedBrowser;
-
-  yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
-  // update the thumbnail to be stale, then re-request it.  The server will
-  // return a 200 response and a red thumbnail - so that new thumbnail should
-  // end up captured.
-  ensureThumbnailStale(URL);
-  let now = Date.now();
-  PageThumbs.captureIfStale(URL).then(() => {
-    ok(getThumbnailModifiedTime(URL) >= now, "modified time should be >= now");
-    // the captureIfStale request saw a 200 response with the red body, so we
-    // expect to see the red version here.
-    retrieveImageDataForURL(URL, function ([r, g, b]) {
-      is("" + [r,g,b], "" + [255, 0, 0], "thumbnail is now red");
-      next();
-    });
-  }).then(null, err => {ok(false, "Error in captureIfStale: " + err)});
-  yield undefined; // wait for callback to call 'next'...
-}
-
-/* Check functionality of a foreground capture when there is an error response
-   from the server.
- */
-function foregroundErrorResponseUpdateTest() {
-  const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?fail";
-  yield addTab(URL);
-  yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
-  gBrowser.removeTab(gBrowser.selectedTab);
-  // do it again - the server will return a 400, so the foreground service
-  // should not update it.
-  yield addTab(URL);
-  yield captureAndCheckColor(0, 255, 0, "we still have a green thumbnail");
-}
-
-function foregroundGoodResponseUpdateTest() {
-  const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?ok";
-  yield addTab(URL);
-  yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
-  gBrowser.removeTab(gBrowser.selectedTab);
-  // do it again - the server will return a 200, so the foreground service
-  // should  update it.
-  yield addTab(URL);
-  yield captureAndCheckColor(255, 0, 0, "we now  have a red thumbnail");
+  yield undefined; // wait for callbacks to call 'next'...
 }

toolkit/components/thumbnails/test/head.js

@@ -52,13 +52,8 @@ let TestRunner = {
   next: function () {
     try {
       let value = TestRunner._iter.next();
-      if (value && typeof value.then == "function") {
-        value.then(result => {
-          next(result);
-        }, error => {
-          ok(false, error + "\n" + error.stack);
-        });
-      }
+      if (value && typeof value.then == "function")
+        value.then(next);
     } catch (e if e instanceof StopIteration) {
       finish();
     }
@@ -134,33 +129,20 @@ function captureAndCheckColor(aRed, aGreen, aBlue, aMessage) {
 function retrieveImageDataForURL(aURL, aCallback) {
   let width = 100, height = 100;
   let thumb = PageThumbs.getThumbnailURL(aURL, width, height);
-  // create a tab with a chrome:// URL so it can host the thumbnail image.
-  // Note that we tried creating the element directly in the top-level chrome
-  // document, but this caused a strange problem:
-  // * call this with the url of an image.
-  // * immediately change the image content.
-  // * call this again with the same url (now holding different content)
-  // The original image data would be used.  Maybe the img hadn't been
-  // collected yet and the platform noticed the same URL, so reused the
-  // content?  Not sure - but this solves the problem.
-  addTab("chrome://global/content/mozilla.xhtml", () => {
-    let doc = gBrowser.selectedBrowser.contentDocument;
-    let htmlns = "http://www.w3.org/1999/xhtml";
-    let img = doc.createElementNS(htmlns, "img");
-    img.setAttribute("src", thumb);
 
-    whenLoaded(img, function () {
-      let canvas = document.createElementNS(htmlns, "canvas");
-      canvas.setAttribute("width", width);
-      canvas.setAttribute("height", height);
+  let htmlns = "http://www.w3.org/1999/xhtml";
+  let img = document.createElementNS(htmlns, "img");
+  img.setAttribute("src", thumb);
 
-      // Draw the image to a canvas and compare the pixel color values.
-      let ctx = canvas.getContext("2d");
-      ctx.drawImage(img, 0, 0, width, height);
-      let result = ctx.getImageData(0, 0, 100, 100).data;
-      gBrowser.removeTab(gBrowser.selectedTab);
-      aCallback(result);
-    });
+  whenLoaded(img, function () {
+    let canvas = document.createElementNS(htmlns, "canvas");
+    canvas.setAttribute("width", width);
+    canvas.setAttribute("height", height);
+
+    // Draw the image to a canvas and compare the pixel color values.
+    let ctx = canvas.getContext("2d");
+    ctx.drawImage(img, 0, 0, width, height);
+    aCallback(ctx.getImageData(0, 0, 100, 100).data);
   });
 }
 

toolkit/components/thumbnails/test/thumbnails_update.sjs

@@ -1,39 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// This server-side script primarily must return different *content* for the
-// second request than it did for the first.
-// Also, it should be able to return an error response when requested for the
-// second response.
-// So the basic tests will be to grab the thumbnail, then request it to be
-// grabbed again:
-// * If the second request succeeded, the new thumbnail should exist.
-// * If the second request is an error, the new thumbnail should be ignored.
-
-function handleRequest(aRequest, aResponse) {
-  aResponse.setHeader("Content-Type", "text/html;charset=utf-8", false);
-  // we want to disable gBrowserThumbnails on-load capture for these responses,
-  // so set as a "no-store" response.
-  aResponse.setHeader("Cache-Control", "no-store");
-
-  let doneError = getState(aRequest.queryString);
-  if (!doneError) {
-    // first request - return a response with a green body and 200 response.
-    aResponse.setStatusLine(aRequest.httpVersion, 200, "OK - It's green");
-    aResponse.write("<html><body bgcolor=00ff00></body></html>");
-    // set the  state so the next request does the "second request" thing below.
-    setState(aRequest.queryString, "yep");
-  } else {
-    // second request - this will be done by the b/g service.
-    // We always return a red background, but depending on the query string we
-    // return either a 200 or 401 response.
-    if (aRequest.queryString == "fail")
-      aResponse.setStatusLine(aRequest.httpVersion, 401, "Oh no you don't");
-    else
-      aResponse.setStatusLine(aRequest.httpVersion, 200, "OK - It's red");
-    aResponse.write("<html><body bgcolor=ff0000></body></html>");
-    // reset the error state incase this ends up being reused for the
-    // same url and querystring.
-    setState(aRequest.queryString, "");
-  }
-}