Bug 1239119 - BackgroundPageThumbs captureIfMissing promise resolves when task is truly done r=ursula
authorOlivier Yiptong <olivier@olivieryiptong.com>
Tue, 08 Mar 2016 14:20:51 -0500
changeset 291403 d0704310fc2fc7cc7f0754df7b2468f9af15b91f
parent 291291 b6ea6a3bb8a6fc355b46403919d8c70e798c7007
child 291404 c8728788af45309519fa8f54fd982f504f06296d
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersursula
bugs1239119
milestone48.0a1
Bug 1239119 - BackgroundPageThumbs captureIfMissing promise resolves when task is truly done r=ursula MozReview-Commit-ID: LmN9Phdrspv
toolkit/components/thumbnails/BackgroundPageThumbs.jsm
toolkit/components/thumbnails/test/browser_thumbnails_bg_bad_url.js
toolkit/components/thumbnails/test/browser_thumbnails_bg_basic.js
toolkit/components/thumbnails/test/browser_thumbnails_bg_crash_during_capture.js
toolkit/components/thumbnails/test/browser_thumbnails_bg_redirect.js
toolkit/components/thumbnails/test/browser_thumbnails_bg_timeout.js
toolkit/components/thumbnails/test/head.js
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -83,33 +83,54 @@ const BackgroundPageThumbs = {
 
   /**
    * Asynchronously captures a thumbnail of the given URL if one does not
    * already exist.  Otherwise does nothing.
    *
    * @param url      The URL to capture.
    * @param options  An optional object that configures the capture.  See
    *                 capture() for description.
-   * @return {Promise} Promise;
+   * @return {Promise} A Promise that resolves when this task completes
    */
   captureIfMissing: Task.async(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.
     let exists = yield PageThumbsStorage.fileExistsForURL(url);
     if (exists) {
       if(options.onDone){
         options.onDone(url);
       }
       return url;
     }
+    let thumbPromise = new Promise((resolve, reject) => {
+      function observe(subject, topic, data) { // jshint ignore:line
+        if (data === url) {
+          switch(topic) {
+            case "page-thumbnail:create":
+              resolve();
+              break;
+            case "page-thumbnail:error":
+              reject(new Error("page-thumbnail:error"));
+              break;
+          }
+          Services.obs.removeObserver(observe, "page-thumbnail:create");
+          Services.obs.removeObserver(observe, "page-thumbnail:error");
+        }
+      }
+      Services.obs.addObserver(observe, "page-thumbnail:create", false);
+      Services.obs.addObserver(observe, "page-thumbnail:error", false);
+    });
     try{
       this.capture(url, options);
+      yield thumbPromise;
     } catch (err) {
-      options.onDone(url);
+      if (options.onDone) {
+        options.onDone(url);
+      }
       throw err;
     }
     return url;
   }),
 
   /**
    * Ensures that initialization of the thumbnail browser's parent window has
    * begun.
@@ -245,16 +266,19 @@ const BackgroundPageThumbs = {
    */
   _onCaptureOrTimeout: function (capture) {
     // Since timeouts start as an item is being processed, only the first
     // item in the queue can be passed to this method.
     if (capture !== this._captureQueue[0])
       throw new Error("The capture should be at the head of the queue.");
     this._captureQueue.shift();
     this._capturesByURL.delete(capture.url);
+    if (capture.doneReason != TEL_CAPTURE_DONE_OK) {
+      Services.obs.notifyObservers(null, "page-thumbnail:error", capture.url);
+    }
 
     // Start the destroy-browser timer *before* processing the capture queue.
     let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
     timer.initWithCallback(this._destroyBrowser.bind(this),
                            this._destroyBrowserTimeout,
                            Ci.nsITimer.TYPE_ONE_SHOT);
     this._destroyBrowserTimer = timer;
 
@@ -280,16 +304,17 @@ Object.defineProperty(this, "BackgroundP
  */
 function Capture(url, captureCallback, options) {
   this.url = url;
   this.captureCallback = captureCallback;
   this.options = options;
   this.id = Capture.nextID++;
   this.creationDate = new Date();
   this.doneCallbacks = [];
+  this.doneReason;
   if (options.onDone)
     this.doneCallbacks.push(options.onDone);
 }
 
 Capture.prototype = {
 
   get pending() {
     return !!this._msgMan;
@@ -366,16 +391,17 @@ Capture.prototype = {
   },
 
   _done: function (data, reason) {
     // Note that _done will be called only once, by either receiveMessage or
     // notify, since it calls destroy here, which cancels the timeout timer and
     // removes the didCapture message listener.
     let { captureCallback, doneCallbacks, options } = this;
     this.destroy();
+    this.doneReason = reason;
 
     if (typeof(reason) != "number") {
       throw new Error("A done reason must be given.");
     }
     tel("CAPTURE_DONE_REASON_2", reason);
     if (data && data.telemetry) {
       // Telemetry is currently disabled in the content process (bug 680508).
       for (let id in data.telemetry) {
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_bad_url.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_bad_url.js
@@ -9,10 +9,15 @@ function* runTests() {
     onDone: function onDone(capturedURL) {
       is(capturedURL, url, "Captured URL should be URL passed to capture");
       is(numCalls++, 0, "onDone should be called only once");
       ok(!thumbnailExists(url),
          "Capture failed so thumbnail should not be cached");
       next();
     },
   });
-  yield true;
+  yield new Promise(resolve => {
+    bgAddPageThumbObserver(url).catch(function(err) {
+      ok(true, "page-thumbnail error produced");
+      resolve();
+    });
+  });
 }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_basic.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_basic.js
@@ -1,13 +1,20 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function* runTests() {
   let url = "http://www.example.com/";
   ok(!thumbnailExists(url), "Thumbnail should not be cached yet.");
 
+  let capturePromise = new Promise(resolve => {
+    bgAddPageThumbObserver(url).then(() => {
+      ok(true, `page-thumbnail created for ${url}`);
+      resolve();
+    });
+  });
   let capturedURL = yield bgCapture(url);
   is(capturedURL, url, "Captured URL should be URL passed to capture");
+  yield capturePromise;
 
   ok(thumbnailExists(url), "Thumbnail should be cached after capture");
   removeThumbnail(url);
 }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_crash_during_capture.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_crash_during_capture.js
@@ -24,13 +24,26 @@ function* runTests() {
   bgCapture(goodUrl, { onDone: () => {
     ok(sawWaitUrlCapture, "waitUrl capture should have finished first");
     ok(thumbnailExists(goodUrl), "We should have recovered and completed the 2nd capture after the crash");
     removeThumbnail(goodUrl);
     // Test done.
     ok(crashObserver.crashed, "Saw a crash from this test");
     next();
   }});
+  let crashPromise = new Promise(resolve => {
+    bgAddPageThumbObserver(waitUrl).catch(function(err) {
+      ok(true, `page-thumbnail error thrown for ${waitUrl}`);
+      resolve();
+    });
+  });
+  let capturePromise = new Promise(resolve => {
+    bgAddPageThumbObserver(goodUrl).then(() => {
+      ok(true, `page-thumbnail created for ${goodUrl}`);
+      resolve();
+    });
+  });
 
   info("Crashing the thumbnail content process.");
   mm.sendAsyncMessage("thumbnails-test:crash");
-  yield true;
+  yield crashPromise;
+  yield capturePromise;
 }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_redirect.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_redirect.js
@@ -5,19 +5,35 @@ function* runTests() {
   let finalURL = "http://example.com/redirected";
   let originalURL = bgTestPageURL({ redirect: finalURL });
 
   ok(!thumbnailExists(originalURL),
      "Thumbnail file for original URL should not exist yet.");
   ok(!thumbnailExists(finalURL),
      "Thumbnail file for final URL should not exist yet.");
 
+  let captureOriginalPromise = new Promise(resolve => {
+    bgAddPageThumbObserver(originalURL).then(() => {
+      ok(true, `page-thumbnail created for ${originalURL}`);
+      resolve();
+    });
+  });
+
+  let captureFinalPromise = new Promise(resolve => {
+    bgAddPageThumbObserver(finalURL).then(() => {
+      ok(true, `page-thumbnail created for ${finalURL}`);
+      resolve();
+    });
+  });
+
   let capturedURL = yield bgCapture(originalURL);
   is(capturedURL, originalURL,
      "Captured URL should be URL passed to capture");
+  yield captureOriginalPromise;
+  yield captureFinalPromise;
   ok(thumbnailExists(originalURL),
      "Thumbnail for original URL should be cached");
   ok(thumbnailExists(finalURL),
      "Thumbnail for final URL should be cached");
 
   removeThumbnail(originalURL);
   removeThumbnail(finalURL);
 }
--- a/toolkit/components/thumbnails/test/browser_thumbnails_bg_timeout.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_timeout.js
@@ -10,10 +10,15 @@ function* runTests() {
     onDone: function onDone(capturedURL) {
       is(capturedURL, url, "Captured URL should be URL passed to capture");
       is(numCalls++, 0, "onDone should be called only once");
       ok(!thumbnailExists(url),
          "Capture timed out so thumbnail should not be cached");
       next();
     },
   });
-  yield true;
+  yield new Promise(resolve => {
+    bgAddPageThumbObserver(url).catch(function(err) {
+      ok(true, `page-thumbnail error thrown for ${url}`);
+      resolve();
+    });
+  });
 }
--- a/toolkit/components/thumbnails/test/head.js
+++ b/toolkit/components/thumbnails/test/head.js
@@ -272,16 +272,37 @@ function bgCaptureWithMethod(aMethodName
   BackgroundPageThumbs[aMethodName](aURL, aOptions);
 }
 
 function bgTestPageURL(aOpts = {}) {
   let TEST_PAGE_URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_background.sjs";
   return TEST_PAGE_URL + "?" + encodeURIComponent(JSON.stringify(aOpts));
 }
 
+function bgAddPageThumbObserver(url) {
+  return new Promise((resolve, reject) => {
+    function observe(subject, topic, data) { // jshint ignore:line
+      if (data === url) {
+        switch(topic) {
+          case "page-thumbnail:create":
+            resolve();
+            break;
+          case "page-thumbnail:error":
+            reject(new Error("page-thumbnail:error"));
+            break;
+        }
+        Services.obs.removeObserver(observe, "page-thumbnail:create");
+        Services.obs.removeObserver(observe, "page-thumbnail:error");
+      }
+    }
+    Services.obs.addObserver(observe, "page-thumbnail:create", false);
+    Services.obs.addObserver(observe, "page-thumbnail:error", false);
+  });
+}
+
 function bgAddCrashObserver() {
   let crashed = false;
   Services.obs.addObserver(function crashObserver(subject, topic, data) {
     is(topic, 'ipc:content-shutdown', 'Received correct observer topic.');
     ok(subject instanceof Components.interfaces.nsIPropertyBag2,
        'Subject implements nsIPropertyBag2.');
     // we might see this called as the process terminates due to previous tests.
     // We are only looking for "abnormal" exits...