From ae1f973204dfba781204d003b2c77a9e60234d78 Mon Sep 17 00:00:00 2001 From: Brendan Dahl Date: Wed, 5 Jun 2013 12:28:31 -0700 Subject: [PATCH 1/2] Use A+ spec compatible promises. --- src/api.js | 14 +-- src/evaluator.js | 4 +- src/obj.js | 55 ++++----- src/util.js | 278 +++++++++++++++++++++++++----------------- test/driver.js | 17 ++- test/unit/api_spec.js | 48 ++++---- web/viewer.js | 22 ++-- 7 files changed, 240 insertions(+), 198 deletions(-) diff --git a/src/api.js b/src/api.js index 91e771a5d..ae457fc7a 100644 --- a/src/api.js +++ b/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 - // 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({ + this.progressCallback({ loaded: data.loaded, total: data.total }); diff --git a/src/evaluator.js b/src/evaluator.js index b031c36a1..d30865f82 100644 --- a/src/evaluator.js +++ b/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, diff --git a/src/obj.js b/src/obj.js index 9d787faf8..9df41af1c 100644 --- a/src/obj.js +++ b/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 - * object *if* it is created. + * Ensures there is an object defined for `objId`. */ - 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. - if (objs[objId]) { - objs[objId].resolve(data); - } else { - this.ensureObj(objId, data); - } - }, - - onData: function PDFObjects_onData(objId, callback) { - this.ensureObj(objId).onData(callback); + obj.resolved = true; + obj.data = data; + obj.promise.resolve(data); }, 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; - if (!objs[objId]) { - return false; - } else { - return objs[objId].hasData; - } + return this.isResolved(objId); }, /** @@ -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 = {}; } diff --git a/src/util.js b/src/util.js index eae731d50..31f119366 100644 --- a/src/util.js +++ b/src/util.js @@ -649,41 +649,129 @@ function isPDFFunction(v) { } /** - * 'Promise' object. - * Each object that is stored in PDFObjects is based on a Promise object that - * contains the status of the object and the data. There might be situations - * where a function wants to use the value of an object, but it isn't ready at - * 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 - * the callback once the object gets resolved. - * 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. + * The following promise implementation tries to generally implment the + * Promise/A+ spec. Some notable differences from other promise libaries are: + * - There currently isn't a seperate deferred and promise object. + * - Unhandled rejections eventually show an error if they aren't handled. + * - Progress events are supported. + * + * Based off of the work in: + * https://bugzilla.mozilla.org/show_bug.cgi?id=810490 */ var Promise = PDFJS.Promise = (function PromiseClosure() { - var EMPTY_PROMISE = {}; + var STATUS_PENDING = 0; + var STATUS_RESOLVED = 1; + var STATUS_REJECTED = 2; + var STATUS_PROGRESS = 3; - /** - * If `data` is passed in this constructor, the promise is created resolved. - * If there isn't data, it isn't resolved at the beginning. - */ - function Promise(name, data) { - this.name = name; - this.isRejected = false; - this.error = null; - this.exception = null; - // If you build a promise and pass in some data it's already resolved. - if (data !== null && data !== undefined) { - this.isResolved = true; - this._data = data; - this.hasData = true; - } else { - this.isResolved = false; - this._data = EMPTY_PROMISE; + // 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 (promise._status !== STATUS_PROGRESS) { + promise._handlers = []; + } + + if (this.running) { + return; + } + this.running = true; + + setTimeout(this.runHandlers.bind(this), 0); + }, + + runHandlers: function runHandlers() { + while (this.handlers.length > 0) { + var handler = this.handlers.shift(); + + var nextStatus = handler.thisPromise._status; + var nextValue = handler.thisPromise._value; + + try { + if (nextStatus === STATUS_RESOLVED) { + if (typeof(handler.onResolve) == 'function') { + nextValue = handler.onResolve(nextValue); + } + } else if (nextStatus === STATUS_PROGRESS) { + if (typeof(handler.onProgress) === 'function') { + nextValue = handler.onProgress(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.pendingRejectionCheck = true; + setTimeout(function rejectionCheck() { + 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); } - this.callbacks = []; - this.errbacks = []; - this.progressbacks = []; + }; + + function Promise() { + this._status = STATUS_PENDING; + this._handlers = []; } /** * Builds a promise that is resolved when all the passed in promises are @@ -700,7 +788,7 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { return deferred; } function reject(reason) { - if (deferred.isRejected) { + if (deferred._status === STATUS_REJECTED) { return; } results = []; @@ -710,7 +798,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 +810,68 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { } return deferred; }; - Promise.prototype = { - hasData: false, - set data(value) { - if (value === undefined) { + Promise.prototype = { + _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); + if (status == STATUS_RESOLVED && + value && typeof(value.then) === 'function') { + value.then(this._updateStatus.bind(this, STATUS_RESOLVED), + this._updateStatus.bind(this, STATUS_REJECTED)); + return; } + + this._status = status; + this._value = value; + + if (status === STATUS_REJECTED && this._handlers.length === 0) { + this._unhandledRejection = true; + HandlerManager.addUnhandledRejection(this); + } + + HandlerManager.scheduleHandlers(this); }, - get data() { - if (this._data === EMPTY_PROMISE) { - error('Promise ' + this.name + ': Cannot get data that isn\'t set'); - } - return this._data; + get isResolved() { + return this._status === STATUS_RESOLVED; }, - onData: function Promise_onData(callback) { - if (this._data !== EMPTY_PROMISE) { - callback(this._data); - } else { - this.onDataCallback = callback; - } + get isRejected() { + return this._status === STATUS_REJECTED; }, - resolve: function Promise_resolve(data) { - if (this.isResolved) { - error('A Promise can be resolved only once ' + this.name); - } - if (this.isRejected) { - error('The Promise was already rejected ' + this.name); - } - - this.isResolved = true; - 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); - } + resolve: function Promise_resolve(value) { + this._updateStatus(STATUS_RESOLVED, value); }, - progress: function Promise_progress(data) { - var callbacks = this.progressbacks; - for (var i = 0, ii = callbacks.length; i < ii; i++) { - callbacks[i].call(null, data); - } + reject: function Promise_reject(reason) { + this._updateStatus(STATUS_REJECTED, reason); }, - reject: function Promise_reject(reason, exception) { - if (this.isRejected) { - 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++) { - errbacks[i].call(null, reason, exception); - } + notify: function Promise_notify(update) { + this._updateStatus(STATUS_PROGRESS, update); }, - then: function Promise_then(callback, errback, progressback) { - // If the promise is already resolved, call the callback directly. - 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) - this.progressbacks.push(progressback); + then: function Promise_then(onResolve, onReject, onProgress) { + var nextPromise = new Promise(); + this._handlers.push({ + thisPromise: this, + onResolve: onResolve, + onReject: onReject, + onProgress: onProgress, + nextPromise: nextPromise + }); + HandlerManager.scheduleHandlers(this); + return nextPromise; } }; diff --git a/test/driver.js b/test/driver.js index 8fc6b249f..50272ea61 100644 --- a/test/driver.js +++ b/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,11 +294,13 @@ function nextPage(task, loadError) { page.stats = new StatTimer(); snapshotCurrentPage(task, error); }); - page.render(renderContext).then(function() { - completeRender(false); - }, - function(error) { - completeRender('render : ' + error); + initPromise.then(function () { + page.render(renderContext).then(function() { + completeRender(false); + }, + function(error) { + completeRender('render : ' + error); + }); }); }, function(error) { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 2904aa439..3d59ecf56 100644 --- a/test/unit/api_spec.js +++ b/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) { - waitsFor(function() { - return promise.isResolved || promise.isRejected; - }, 10000); - } - function expectAfterPromise(promise, successCallback) { - waitsForPromise(promise); - runs(function() { - promise.then(successCallback, - function(error, e) { - // Shouldn't get here. - expect(false).toEqual(true); - }); + function waitsForPromise(promise, successCallback) { + var data; + promise.then(function(val) { + data = val; + successCallback(data); + }, + function(error) { + // 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() { - promise.then(function(data) { doc = data; }); + waitsForPromise(promise, function(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() { - promise.then(function(data) { - page = data; - }); + waitsForPromise(promise, function(data) { + page = data; }); + it('gets ref', function() { expect(page.ref).toEqual({num: 15, gen: 0}); }); diff --git a/web/viewer.js b/web/viewer.js index 4ec85b5d7..66209cafb 100644 --- a/web/viewer.js +++ b/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) { - // Use a timeout since all the pages may already be extracted and we - // want to start highlighting before finding all the matches. - setTimeout(function() { + if (!(i in this.pendingFindMatches)) { + this.pendingFindMatches[i] = true; + this.extractTextPromises[i].then(function(pageIdx) { + 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); } ); }, From 7764dd95c02c14211ec225dda911ec9b6b29ff60 Mon Sep 17 00:00:00 2001 From: Brendan Dahl Date: Thu, 6 Jun 2013 15:48:54 -0700 Subject: [PATCH 2/2] Remove progress events. --- src/util.js | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/util.js b/src/util.js index 31f119366..248be72e8 100644 --- a/src/util.js +++ b/src/util.js @@ -653,7 +653,6 @@ function isPDFFunction(v) { * Promise/A+ spec. Some notable differences from other promise libaries are: * - There currently isn't a seperate deferred and promise object. * - Unhandled rejections eventually show an error if they aren't handled. - * - Progress events are supported. * * Based off of the work in: * https://bugzilla.mozilla.org/show_bug.cgi?id=810490 @@ -662,7 +661,6 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { var STATUS_PENDING = 0; var STATUS_RESOLVED = 1; var STATUS_REJECTED = 2; - var STATUS_PROGRESS = 3; // In an attempt to avoid silent exceptions, unhandled rejections are // tracked and if they aren't handled in a certain amount of time an @@ -681,9 +679,7 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { } this.handlers = this.handlers.concat(promise._handlers); - if (promise._status !== STATUS_PROGRESS) { - promise._handlers = []; - } + promise._handlers = []; if (this.running) { return; @@ -705,10 +701,6 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { if (typeof(handler.onResolve) == 'function') { nextValue = handler.onResolve(nextValue); } - } else if (nextStatus === STATUS_PROGRESS) { - if (typeof(handler.onProgress) === 'function') { - nextValue = handler.onProgress(nextValue); - } } else if (typeof(handler.onReject) === 'function') { nextValue = handler.onReject(nextValue); nextStatus = STATUS_RESOLVED; @@ -857,17 +849,12 @@ var Promise = PDFJS.Promise = (function PromiseClosure() { this._updateStatus(STATUS_REJECTED, reason); }, - notify: function Promise_notify(update) { - this._updateStatus(STATUS_PROGRESS, update); - }, - - then: function Promise_then(onResolve, onReject, onProgress) { + then: function Promise_then(onResolve, onReject) { var nextPromise = new Promise(); this._handlers.push({ thisPromise: this, onResolve: onResolve, onReject: onReject, - onProgress: onProgress, nextPromise: nextPromise }); HandlerManager.scheduleHandlers(this);