Bug 1033972 - BackgroundPageThumbs.captureIfMissing() and PageThumbs.captureAndStoreIfStale() both incorrectly use PromiseWorker. r=adw
authorBlair McBride <bmcbride@mozilla.com>
Mon, 07 Jul 2014 14:28:00 +1200
changeset 192536 54cb0b2e118b617fd2ca61811b54cd461ee57f10
parent 192535 dc44c1bd01a40f372f274ef75c842947dcda6498
child 192537 9b0206fad5f231aed05bd3017e2dfc81b029986c
child 192596 16fab85292412a00cb7602de9d5f16f5b4f7f254
push id27089
push usercbook@mozilla.com
push dateMon, 07 Jul 2014 12:44:49 +0000
treeherdermozilla-central@9b0206fad5f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1033972
milestone33.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 1033972 - BackgroundPageThumbs.captureIfMissing() and PageThumbs.captureAndStoreIfStale() both incorrectly use PromiseWorker. r=adw
toolkit/components/thumbnails/BackgroundPageThumbs.jsm
toolkit/components/thumbnails/PageThumbs.jsm
toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
toolkit/components/thumbnails/test/browser_thumbnails_update.js
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -81,17 +81,17 @@ const BackgroundPageThumbs = {
    * @param options  An optional object that configures the capture.  See
    *                 capture() for description.
    */
   captureIfMissing: function (url, options={}) {
     // The fileExistsForURL call is an optimization, potentially but unlikely
     // incorrect, and no big deal when it is.  After the capture is done, we
     // atomically test whether the file exists before writing it.
     PageThumbsStorage.fileExistsForURL(url).then(exists => {
-      if (exists.ok) {
+      if (exists) {
         if (options.onDone)
           options.onDone(url);
         return;
       }
       this.capture(url, options);
     }, err => {
       if (options.onDone)
         options.onDone(url);
--- a/toolkit/components/thumbnails/PageThumbs.jsm
+++ b/toolkit/components/thumbnails/PageThumbs.jsm
@@ -290,17 +290,17 @@ this.PageThumbs = {
    * such notifications if they want to be notified of an updated thumbnail.
    *
    * @param aBrowser The content window of this browser will be captured.
    * @param aCallback The function to be called when finished (optional).
    */
   captureAndStoreIfStale: function PageThumbs_captureAndStoreIfStale(aBrowser, aCallback) {
     let url = aBrowser.currentURI.spec;
     PageThumbsStorage.isFileRecentForURL(url).then(recent => {
-      if (!recent.ok &&
+      if (!recent &&
           // Careful, the call to PageThumbsStorage is async, so the browser may
           // have navigated away from the URL or even closed.
           aBrowser.currentURI &&
           aBrowser.currentURI.spec == url) {
         this.captureAndStore(aBrowser, aCallback);
       } else if (aCallback) {
         aCallback(true);
       }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
@@ -1,24 +1,35 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function runTests() {
+  let numNotifications = 0;
+  function observe(subject, topic, data) {
+    is(topic, "page-thumbnail:create", "got expected topic");
+    numNotifications += 1;
+  }
+
+  Services.obs.addObserver(observe, "page-thumbnail:create", false);
+
   let url = "http://example.com/";
   let file = thumbnailFile(url);
   ok(!file.exists(), "Thumbnail file should not already exist.");
 
   let capturedURL = yield bgCaptureIfMissing(url);
+  is(numNotifications, 1, "got notification of item being created.");
   is(capturedURL, url, "Captured URL should be URL passed to capture");
   ok(file.exists(url), "Thumbnail should be cached after capture");
 
   let past = Date.now() - 1000000000;
   let pastFudge = past + 30000;
   file.lastModifiedTime = past;
   ok(file.lastModifiedTime < pastFudge, "Last modified time should stick!");
   capturedURL = yield bgCaptureIfMissing(url);
+  is(numNotifications, 1, "still only 1 notification of item being created.");
   is(capturedURL, url, "Captured URL should be URL passed to second capture");
   ok(file.exists(), "Thumbnail should remain cached after second capture");
   ok(file.lastModifiedTime < pastFudge,
      "File should not have been overwritten");
 
   file.remove(false);
+  Services.obs.removeObserver(observe, "page-thumbnail:create");
 }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_update.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_update.js
@@ -62,24 +62,25 @@ function simpleCaptureTest() {
   let browser = gBrowser.selectedBrowser;
 
   // Capture the screenshot.
   PageThumbs.captureAndStore(browser, function () {
     // 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
     // a new capture.
-    PageThumbs.captureAndStoreIfStale(browser);
-    is(numNotifications, 1, "still only 1 notification of item being created.");
+    PageThumbs.captureAndStoreIfStale(browser, function() {
+      is(numNotifications, 1, "still only 1 notification of item being created.");
 
-    ensureThumbnailStale(URL);
-    // Ask for it to be updated.
-    PageThumbs.captureAndStoreIfStale(browser);
-    // But it's async, so wait - our observer above will call next() when
-    // the notification comes.
+      ensureThumbnailStale(URL);
+      // Ask for it to be updated.
+      PageThumbs.captureAndStoreIfStale(browser);
+      // 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 captureAndStoreIfStale when there is an error response
    from the server.
  */
 function capIfStaleErrorResponseUpdateTest() {