Merge pull request #3340 from brendandahl/chainable-promises2

Use A+ spec compatible promises.

src/api.js

@@ -48,7 +48,8 @@
  */
 PDFJS.getDocument = function getDocument(source,
                                          pdfDataRangeTransport,
-                                         passwordCallback) {
+                                         passwordCallback,
+                                         progressCallback) {
   var workerInitializedPromise, workerReadyPromise, transport;
 
   if (typeof source === 'string') {
@@ -76,7 +77,7 @@ PDFJS.getDocument = function getDocument(source,
   workerInitializedPromise = new PDFJS.Promise();
   workerReadyPromise = new PDFJS.Promise();
   transport = new WorkerTransport(workerInitializedPromise,
-      workerReadyPromise, pdfDataRangeTransport);
+      workerReadyPromise, pdfDataRangeTransport, progressCallback);
   workerInitializedPromise.then(function transportInitialized() {
     transport.passwordCallback = passwordCallback;
     transport.fetchDocument(params);
@@ -480,10 +481,11 @@ var PDFPageProxy = (function PDFPageProxyClosure() {
  */
 var WorkerTransport = (function WorkerTransportClosure() {
   function WorkerTransport(workerInitializedPromise, workerReadyPromise,
-      pdfDataRangeTransport) {
+      pdfDataRangeTransport, progressCallback) {
     this.pdfDataRangeTransport = pdfDataRangeTransport;
 
     this.workerReadyPromise = workerReadyPromise;
+    this.progressCallback = progressCallback;
     this.commonObjs = new PDFObjects();
 
     this.pageCache = [];
@@ -705,11 +707,7 @@ var WorkerTransport = (function WorkerTransportClosure() {
       }, this);
 
       messageHandler.on('DocProgress', function transportDocProgress(data) {
-        // TODO(mack): The progress event should be resolved on a different
+        this.progressCallback({
-        // promise that tracks progress of whole file, since workerReadyPromise
-        // is for file being ready to render, not for when file is fully
-        // downloaded
-        this.workerReadyPromise.progress({
           loaded: data.loaded,
           total: data.total
         });

src/evaluator.js

@@ -533,7 +533,7 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
       // keep track of each font we translated so the caller can
       // load them asynchronously before calling display on a page
       font.loadedName = 'g_font_' + this.uniquePrefix +
-                        (this.idCounters.font + 1);
+                        (++this.idCounters.font);
 
       if (!font.translated) {
         var translated;
@@ -567,14 +567,12 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
           }
           font.translated.charProcOperatorList = charProcOperatorList;
           font.loaded = true;
-          ++this.idCounters.font;
           promise.resolve({
             font: font,
             dependencies: dependencies
           });
         }.bind(this));
       } else {
-        ++this.idCounters.font;
         font.loaded = true;
         promise.resolve({
           font: font,

src/obj.js

@@ -1072,13 +1072,20 @@ var PDFObjects = (function PDFObjectsClosure() {
   PDFObjects.prototype = {
     /**
      * Internal function.
-     * Ensures there is an object defined for `objId`. Stores `data` on the
+     * Ensures there is an object defined for `objId`.
-     * object *if* it is created.
      */
-    ensureObj: function PDFObjects_ensureObj(objId, data) {
+    ensureObj: function PDFObjects_ensureObj(objId) {
       if (this.objs[objId])
         return this.objs[objId];
-      return this.objs[objId] = new Promise(objId, data);
+
+      var obj = {
+        promise: new Promise(objId),
+        data: null,
+        resolved: false
+      };
+      this.objs[objId] = obj;
+
+      return obj;
     },
 
     /**
@@ -1094,7 +1101,7 @@ var PDFObjects = (function PDFObjectsClosure() {
       // If there is a callback, then the get can be async and the object is
       // not required to be resolved right now
       if (callback) {
-        this.ensureObj(objId).then(callback);
+        this.ensureObj(objId).promise.then(callback);
         return null;
       }
 
@@ -1104,7 +1111,7 @@ var PDFObjects = (function PDFObjectsClosure() {
 
       // If there isn't an object yet or the object isn't resolved, then the
       // data isn't ready yet!
-      if (!obj || !obj.isResolved)
+      if (!obj || !obj.resolved)
         error('Requesting object that isn\'t resolved yet ' + objId);
 
       return obj.data;
@@ -1114,36 +1121,25 @@ var PDFObjects = (function PDFObjectsClosure() {
      * Resolves the object `objId` with optional `data`.
      */
     resolve: function PDFObjects_resolve(objId, data) {
-      var objs = this.objs;
+      var obj = this.ensureObj(objId);
 
-      // In case there is a promise already on this object, just resolve it.
+      obj.resolved = true;
-      if (objs[objId]) {
+      obj.data = data;
-        objs[objId].resolve(data);
+      obj.promise.resolve(data);
-      } else {
-        this.ensureObj(objId, data);
-      }
-    },
-
-    onData: function PDFObjects_onData(objId, callback) {
-      this.ensureObj(objId).onData(callback);
     },
 
     isResolved: function PDFObjects_isResolved(objId) {
       var objs = this.objs;
+
       if (!objs[objId]) {
         return false;
       } else {
-        return objs[objId].isResolved;
+        return objs[objId].resolved;
       }
     },
 
     hasData: function PDFObjects_hasData(objId) {
-      var objs = this.objs;
+      return this.isResolved(objId);
-      if (!objs[objId]) {
-        return false;
-      } else {
-        return objs[objId].hasData;
-      }
     },
 
     /**
@@ -1151,22 +1147,13 @@ var PDFObjects = (function PDFObjectsClosure() {
      */
     getData: function PDFObjects_getData(objId) {
       var objs = this.objs;
-      if (!objs[objId] || !objs[objId].hasData) {
+      if (!objs[objId] || !objs[objId].resolved) {
         return null;
       } else {
         return objs[objId].data;
       }
     },
 
-    /**
-     * Sets the data of an object but *doesn't* resolve it.
-     */
-    setData: function PDFObjects_setData(objId, data) {
-      // Watchout! If you call `this.ensureObj(objId, data)` you're going to
-      // create a *resolved* promise which shouldn't be the case!
-      this.ensureObj(objId).data = data;
-    },
-
     clear: function PDFObjects_clear() {
       this.objs = {};
     }

src/util.js

@@ -649,41 +649,121 @@ function isPDFFunction(v) {
 }
 
 /**
- * 'Promise' object.
+ * The following promise implementation tries to generally implment the
- * Each object that is stored in PDFObjects is based on a Promise object that
+ * Promise/A+ spec. Some notable differences from other promise libaries are:
- * contains the status of the object and the data. There might be situations
+ * - There currently isn't a seperate deferred and promise object.
- * where a function wants to use the value of an object, but it isn't ready at
+ * - Unhandled rejections eventually show an error if they aren't handled.
- * that time. To get a notification, once the object is ready to be used, s.o.
+ *
- * can add a callback using the `then` method on the promise that then calls
+ * Based off of the work in:
- * the callback once the object gets resolved.
+ * https://bugzilla.mozilla.org/show_bug.cgi?id=810490
- * A promise can get resolved only once and only once the data of the promise
- * can be set. If any of these happens twice or the data is required before
- * it was set, an exception is throw.
  */
 var Promise = PDFJS.Promise = (function PromiseClosure() {
-  var EMPTY_PROMISE = {};
+  var STATUS_PENDING = 0;
+  var STATUS_RESOLVED = 1;
+  var STATUS_REJECTED = 2;
+
+  // In an attempt to avoid silent exceptions, unhandled rejections are
+  // tracked and if they aren't handled in a certain amount of time an
+  // error is logged.
+  var REJECTION_TIMEOUT = 500;
+
+  var HandlerManager = {
+    handlers: [],
+    running: false,
+    unhandledRejections: [],
+    pendingRejectionCheck: false,
+
+    scheduleHandlers: function scheduleHandlers(promise) {
+      if (promise._status == STATUS_PENDING) {
+        return;
+      }
 
-  /**
+      this.handlers = this.handlers.concat(promise._handlers);
-   * If `data` is passed in this constructor, the promise is created resolved.
+      promise._handlers = [];
-   * If there isn't data, it isn't resolved at the beginning.
+
-   */
+      if (this.running) {
-  function Promise(name, data) {
+        return;
-    this.name = name;
+      }
-    this.isRejected = false;
+      this.running = true;
-    this.error = null;
+
-    this.exception = null;
+      setTimeout(this.runHandlers.bind(this), 0);
-    // If you build a promise and pass in some data it's already resolved.
+    },
-    if (data !== null && data !== undefined) {
+
-      this.isResolved = true;
+    runHandlers: function runHandlers() {
-      this._data = data;
+      while (this.handlers.length > 0) {
-      this.hasData = true;
+        var handler = this.handlers.shift();
-    } else {
+
-      this.isResolved = false;
+        var nextStatus = handler.thisPromise._status;
-      this._data = EMPTY_PROMISE;
+        var nextValue = handler.thisPromise._value;
+
+        try {
+          if (nextStatus === STATUS_RESOLVED) {
+            if (typeof(handler.onResolve) == 'function') {
+              nextValue = handler.onResolve(nextValue);
+            }
+          } else if (typeof(handler.onReject) === 'function') {
+              nextValue = handler.onReject(nextValue);
+              nextStatus = STATUS_RESOLVED;
+
+              if (handler.thisPromise._unhandledRejection) {
+                this.removeUnhandeledRejection(handler.thisPromise);
+              }
+          }
+        } catch (ex) {
+          nextStatus = STATUS_REJECTED;
+          nextValue = ex;
+        }
+
+        handler.nextPromise._updateStatus(nextStatus, nextValue);
+      }
+
+      this.running = false;
+    },
+
+    addUnhandledRejection: function addUnhandledRejection(promise) {
+      this.unhandledRejections.push({
+        promise: promise,
+        time: Date.now()
+      });
+      this.scheduleRejectionCheck();
+    },
+
+    removeUnhandeledRejection: function removeUnhandeledRejection(promise) {
+      promise._unhandledRejection = false;
+      for (var i = 0; i < this.unhandledRejections.length; i++) {
+        if (this.unhandledRejections[i].promise === promise) {
+          this.unhandledRejections.splice(i);
+          i--;
+        }
+      }
+    },
+
+    scheduleRejectionCheck: function scheduleRejectionCheck() {
+      if (this.pendingRejectionCheck) {
+        return;
       }
-    this.callbacks = [];
+      this.pendingRejectionCheck = true;
-    this.errbacks = [];
+      setTimeout(function rejectionCheck() {
-    this.progressbacks = [];
+        this.pendingRejectionCheck = false;
+        var now = Date.now();
+        for (var i = 0; i < this.unhandledRejections.length; i++) {
+          if (now - this.unhandledRejections[i].time > REJECTION_TIMEOUT) {
+            console.error('Unhandled rejection: ' +
+                          this.unhandledRejections[i].promise._value);
+            this.unhandledRejections.splice(i);
+            i--;
+          }
+        }
+        if (this.unhandledRejections.length) {
+          this.scheduleRejectionCheck();
+        }
+      }.bind(this), REJECTION_TIMEOUT);
+    }
+  };
+
+  function Promise() {
+    this._status = STATUS_PENDING;
+    this._handlers = [];
   }
   /**
    * Builds a promise that is resolved when all the passed in promises are
@@ -700,7 +780,7 @@ var Promise = PDFJS.Promise = (function PromiseClosure() {
       return deferred;
     }
     function reject(reason) {
-      if (deferred.isRejected) {
+      if (deferred._status === STATUS_REJECTED) {
         return;
       }
       results = [];
@@ -710,7 +790,7 @@ var Promise = PDFJS.Promise = (function PromiseClosure() {
       var promise = promises[i];
       promise.then((function(i) {
         return function(value) {
-          if (deferred.isRejected) {
+          if (deferred._status === STATUS_REJECTED) {
             return;
           }
           results[i] = value;
@@ -722,102 +802,63 @@ var Promise = PDFJS.Promise = (function PromiseClosure() {
     }
     return deferred;
   };
-  Promise.prototype = {
-    hasData: false,
 
-    set data(value) {
+  Promise.prototype = {
-      if (value === undefined) {
+    _status: null,
+    _value: null,
+    _handlers: null,
+    _unhandledRejection: null,
+
+    _updateStatus: function Promise__updateStatus(status, value) {
+      if (this._status === STATUS_RESOLVED ||
+          this._status === STATUS_REJECTED) {
         return;
       }
-      if (this._data !== EMPTY_PROMISE) {
-        error('Promise ' + this.name +
-              ': Cannot set the data of a promise twice');
-      }
-      this._data = value;
-      this.hasData = true;
-
-      if (this.onDataCallback) {
-        this.onDataCallback(value);
-      }
-    },
 
-    get data() {
+      if (status == STATUS_RESOLVED &&
-      if (this._data === EMPTY_PROMISE) {
+          value && typeof(value.then) === 'function') {
-        error('Promise ' + this.name + ': Cannot get data that isn\'t set');
+        value.then(this._updateStatus.bind(this, STATUS_RESOLVED),
+                   this._updateStatus.bind(this, STATUS_REJECTED));
+        return;
       }
-      return this._data;
-    },
 
-    onData: function Promise_onData(callback) {
+      this._status = status;
-      if (this._data !== EMPTY_PROMISE) {
+      this._value = value;
-        callback(this._data);
-      } else {
-        this.onDataCallback = callback;
-      }
-    },
 
-    resolve: function Promise_resolve(data) {
+      if (status === STATUS_REJECTED && this._handlers.length === 0) {
-      if (this.isResolved) {
+        this._unhandledRejection = true;
-        error('A Promise can be resolved only once ' + this.name);
+        HandlerManager.addUnhandledRejection(this);
-      }
-      if (this.isRejected) {
-        error('The Promise was already rejected ' + this.name);
       }
 
-      this.isResolved = true;
+      HandlerManager.scheduleHandlers(this);
-      this.data = (typeof data !== 'undefined') ? data : null;
-      var callbacks = this.callbacks;
-
-      for (var i = 0, ii = callbacks.length; i < ii; i++) {
-        callbacks[i].call(null, data);
-      }
     },
 
-    progress: function Promise_progress(data) {
+    get isResolved() {
-      var callbacks = this.progressbacks;
+      return this._status === STATUS_RESOLVED;
-      for (var i = 0, ii = callbacks.length; i < ii; i++) {
-        callbacks[i].call(null, data);
-      }
     },
 
-    reject: function Promise_reject(reason, exception) {
+    get isRejected() {
-      if (this.isRejected) {
+      return this._status === STATUS_REJECTED;
-        error('A Promise can be rejected only once ' + this.name);
+    },
-      }
-      if (this.isResolved) {
-        error('The Promise was already resolved ' + this.name);
-      }
-
-      this.isRejected = true;
-      this.error = reason || null;
-      this.exception = exception || null;
-      var errbacks = this.errbacks;
 
-      for (var i = 0, ii = errbacks.length; i < ii; i++) {
+    resolve: function Promise_resolve(value) {
-        errbacks[i].call(null, reason, exception);
+      this._updateStatus(STATUS_RESOLVED, value);
-      }
     },
 
-    then: function Promise_then(callback, errback, progressback) {
+    reject: function Promise_reject(reason) {
-      // If the promise is already resolved, call the callback directly.
+      this._updateStatus(STATUS_REJECTED, reason);
-      if (this.isResolved && callback) {
+    },
-        var data = this.data;
-        callback.call(null, data);
-      } else if (this.isRejected && errback) {
-        var error = this.error;
-        var exception = this.exception;
-        errback.call(null, error, exception);
-      } else {
-        if (callback) {
-          this.callbacks.push(callback);
-        }
-        if (errback) {
-          this.errbacks.push(errback);
-        }
-      }
 
-      if (progressback)
+    then: function Promise_then(onResolve, onReject) {
-        this.progressbacks.push(progressback);
+      var nextPromise = new Promise();
+      this._handlers.push({
+        thisPromise: this,
+        onResolve: onResolve,
+        onReject: onReject,
+        nextPromise: nextPromise
+      });
+      HandlerManager.scheduleHandlers(this);
+      return nextPromise;
     }
   };
 

test/driver.js

@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-/* globals PDFJS, getPdf, combineUrl, StatTimer, SpecialPowers */
+/* globals PDFJS, getPdf, combineUrl, StatTimer, SpecialPowers, Promise */
 
 'use strict';
 
@@ -264,6 +264,7 @@ function nextPage(task, loadError) {
         clear(ctx);
 
         var drawContext, textLayerBuilder;
+        var initPromise = new Promise();
         if (task.type == 'text') {
           // using dummy canvas for pdf context drawing operations
           if (!dummyCanvas) {
@@ -275,10 +276,12 @@ function nextPage(task, loadError) {
 
           page.getTextContent().then(function(textContent) {
             textLayerBuilder.setTextContent(textContent);
+            initPromise.resolve();
           });
         } else {
           drawContext = ctx;
           textLayerBuilder = new NullTextLayerBuilder();
+          initPromise.resolve();
         }
         var renderContext = {
           canvasContext: drawContext,
@@ -291,12 +294,14 @@ function nextPage(task, loadError) {
           page.stats = new StatTimer();
           snapshotCurrentPage(task, error);
         });
+        initPromise.then(function () {
           page.render(renderContext).then(function() {
             completeRender(false);
           },
           function(error) {
             completeRender('render : ' + error);
           });
+        });
       },
       function(error) {
         snapshotCurrentPage(task, 'render : ' + error);

test/unit/api_spec.js

@@ -8,26 +8,25 @@ describe('api', function() {
   // TODO run with worker enabled
   PDFJS.disableWorker = true;
   var basicApiUrl = combineUrl(window.location.href, '../pdfs/basicapi.pdf');
-  function waitsForPromise(promise) {
+  function waitsForPromise(promise, successCallback) {
-    waitsFor(function() {
+    var data;
-      return promise.isResolved || promise.isRejected;
+    promise.then(function(val) {
-    }, 10000);
+      data = val;
-  }
+      successCallback(data);
-  function expectAfterPromise(promise, successCallback) {
+    },
-    waitsForPromise(promise);
+    function(error) {
-    runs(function() {
-      promise.then(successCallback,
-      function(error, e) {
       // Shouldn't get here.
       expect(false).toEqual(true);
     });
-    });
+    waitsFor(function() {
+      return data !== undefined;
+    }, 10000);
   }
   describe('PDFJS', function() {
     describe('getDocument', function() {
       it('creates pdf doc from URL', function() {
         var promise = PDFJS.getDocument(basicApiUrl);
-        expectAfterPromise(promise, function(data) {
+        waitsForPromise(promise, function(data) {
           expect(true).toEqual(true);
         });
       });
@@ -40,10 +39,9 @@ describe('api', function() {
   });
   describe('PDFDocument', function() {
     var promise = PDFJS.getDocument(basicApiUrl);
-    waitsForPromise(promise);
     var doc;
-    runs(function() {
+    waitsForPromise(promise, function(data) {
-      promise.then(function(data) { doc = data; });
+      doc = data;
     });
     it('gets number of pages', function() {
       expect(doc.numPages).toEqual(3);
@@ -53,19 +51,19 @@ describe('api', function() {
     });
     it('gets page', function() {
       var promise = doc.getPage(1);
-      expectAfterPromise(promise, function(data) {
+      waitsForPromise(promise, function(data) {
         expect(true).toEqual(true);
       });
     });
     it('gets destinations', function() {
       var promise = doc.getDestinations();
-      expectAfterPromise(promise, function(data) {
+      waitsForPromise(promise, function(data) {
         // TODO this seems to be broken for the test pdf
       });
     });
     it('gets outline', function() {
       var promise = doc.getOutline();
-      expectAfterPromise(promise, function(outline) {
+      waitsForPromise(promise, function(outline) {
         // Two top level entries.
         expect(outline.length).toEqual(2);
         // Make sure some basic attributes are set.
@@ -76,7 +74,7 @@ describe('api', function() {
     });
     it('gets metadata', function() {
       var promise = doc.getMetadata();
-      expectAfterPromise(promise, function(metadata) {
+      waitsForPromise(promise, function(metadata) {
         expect(metadata.info['Title']).toEqual('Basic API Test');
         expect(metadata.metadata.get('dc:title')).toEqual('Basic API Test');
       });
@@ -89,13 +87,11 @@ describe('api', function() {
         promise.resolve(data);
       });
     });
-    waitsForPromise(promise);
     var page;
-    runs(function() {
+    waitsForPromise(promise, function(data) {
-      promise.then(function(data) {
       page = data;
     });
-    });
+
     it('gets ref', function() {
       expect(page.ref).toEqual({num: 15, gen: 0});
     });

web/viewer.js

@@ -250,6 +250,8 @@ var PDFFindController = {
 
   extractTextPromises: [],
 
+  pendingFindMatches: {},
+
   // If active, find results will be highlighted.
   active: false,
 
@@ -425,13 +427,13 @@ var PDFFindController = {
         this.updatePage(i);
 
         // As soon as the text is extracted start finding the matches.
-        this.extractTextPromises[i].onData(function(pageIdx) {
+        if (!(i in this.pendingFindMatches)) {
-          // Use a timeout since all the pages may already be extracted and we
+          this.pendingFindMatches[i] = true;
-          // want to start highlighting before finding all the matches.
+          this.extractTextPromises[i].then(function(pageIdx) {
-          setTimeout(function() {
+            delete self.pendingFindMatches[pageIdx];
             self.calcFindMatch(pageIdx);
           });
-        });
+        }
       }
     }
 
@@ -1355,7 +1357,12 @@ var PDFView = {
       }
     };
 
-    PDFJS.getDocument(parameters, pdfDataRangeTransport, passwordNeeded).then(
+    function getDocumentProgress(progressData) {
+      self.progress(progressData.loaded / progressData.total);
+    }
+
+    PDFJS.getDocument(parameters, pdfDataRangeTransport, passwordNeeded,
+                      getDocumentProgress).then(
       function getDocumentCallback(pdfDocument) {
         self.load(pdfDocument, scale);
         self.loading = false;
@@ -1390,9 +1397,6 @@ var PDFView = {
         };
         self.error(loadingErrorMessage, moreInfo);
         self.loading = false;
-      },
-      function getDocumentProgress(progressData) {
-        self.progress(progressData.loaded / progressData.total);
       }
     );
   },