From b2948dad76061737fde87666b8dc894e99bec096 Mon Sep 17 00:00:00 2001
From: Blair McBride <bmcbride@mozilla.com>
Date: Mon, 7 Jul 2014 14:28:00 +1200
Subject: [PATCH] Bug 1033972 - BackgroundPageThumbs.captureIfMissing() and
 PageThumbs.captureAndStoreIfStale() both incorrectly use PromiseWorker. r=adw

---
 .../thumbnails/BackgroundPageThumbs.jsm           |  2 +-
 toolkit/components/thumbnails/PageThumbs.jsm      |  2 +-
 .../browser_thumbnails_bg_captureIfMissing.js     | 11 +++++++++++
 .../thumbnails/test/browser_thumbnails_update.js  | 15 ++++++++-------
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
index 4866f3927531..347be5adc289 100644
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -86,7 +86,7 @@ const BackgroundPageThumbs = {
     // 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;
diff --git a/toolkit/components/thumbnails/PageThumbs.jsm b/toolkit/components/thumbnails/PageThumbs.jsm
index e5ff28a9a14c..12983407ebbf 100644
--- a/toolkit/components/thumbnails/PageThumbs.jsm
+++ b/toolkit/components/thumbnails/PageThumbs.jsm
@@ -295,7 +295,7 @@ this.PageThumbs = {
   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 &&
diff --git a/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js b/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
index b5468f6faad4..b6afde0f1388 100644
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_captureIfMissing.js
@@ -2,11 +2,20 @@
  * 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");
 
@@ -15,10 +24,12 @@ function runTests() {
   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");
 }
diff --git a/toolkit/components/thumbnails/test/browser_thumbnails_update.js b/toolkit/components/thumbnails/test/browser_thumbnails_update.js
index 59016edbeda0..523dece88e7d 100644
--- a/toolkit/components/thumbnails/test/browser_thumbnails_update.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_update.js
@@ -67,14 +67,15 @@ function simpleCaptureTest() {
     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'...
 }