From d32321d84f32d1610b92ad8b5f785a20f62bec10 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 7 Nov 2018 14:17:14 +0100 Subject: [PATCH 1/2] Convert `PDFObjects`, in `src/display/api.js`, to an ES6 class Also changes all occurrences of `var` to `const`, and marks internal properties/methods as "private". --- src/display/api.js | 166 ++++++++++++++++++++------------------------- 1 file changed, 75 insertions(+), 91 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 28e76ff57..784fc30ff 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2205,108 +2205,92 @@ class WorkerTransport { } /** - * A PDF document and page is built of many objects. E.g. there are objects - * for fonts, images, rendering code and such. These objects might get processed - * inside of a worker. The `PDFObjects` implements some basic functions to - * manage these objects. + * A PDF document and page is built of many objects. E.g. there are objects for + * fonts, images, rendering code, etc. These objects may get processed inside of + * a worker. This class implements some basic methods to manage these objects. * @ignore */ -var PDFObjects = (function PDFObjectsClosure() { - function PDFObjects() { - this.objs = Object.create(null); +class PDFObjects { + constructor() { + this._objs = Object.create(null); } - PDFObjects.prototype = { - /** - * Internal function. - * Ensures there is an object defined for `objId`. - */ - ensureObj: function PDFObjects_ensureObj(objId) { - if (this.objs[objId]) { - return this.objs[objId]; - } + /** + * Ensures there is an object defined for `objId`. + * @private + */ + _ensureObj(objId) { + if (this._objs[objId]) { + return this._objs[objId]; + } + return this._objs[objId] = { + capability: createPromiseCapability(), + data: null, + resolved: false, + }; + } - var obj = { - capability: createPromiseCapability(), - data: null, - resolved: false, - }; - this.objs[objId] = obj; + /** + * If called *without* callback, this returns the data of `objId` but the + * object needs to be resolved. If it isn't, this method throws. + * + * If called *with* a callback, the callback is called with the data of the + * object once the object is resolved. That means, if you call this method + * and the object is already resolved, the callback gets called right away. + */ + get(objId, callback = null) { + // 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).capability.promise.then(callback); + return null; + } + // If there isn't a callback, the user expects to get the resolved data + // directly. + const obj = this._objs[objId]; + // If there isn't an object yet or the object isn't resolved, then the + // data isn't ready yet! + if (!obj || !obj.resolved) { + throw new Error(`Requesting object that isn't resolved yet ${objId}.`); + } + return obj.data; + } - return obj; - }, + /** + * Resolves the object `objId` with optional `data`. + */ + resolve(objId, data) { + const obj = this._ensureObj(objId); - /** - * If called *without* callback, this returns the data of `objId` but the - * object needs to be resolved. If it isn't, this function throws. - * - * If called *with* a callback, the callback is called with the data of the - * object once the object is resolved. That means, if you call this - * function and the object is already resolved, the callback gets called - * right away. - */ - get: function PDFObjects_get(objId, callback) { - // 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).capability.promise.then(callback); - return null; - } + obj.resolved = true; + obj.data = data; + obj.capability.resolve(data); + } - // If there isn't a callback, the user expects to get the resolved data - // directly. - var obj = this.objs[objId]; + isResolved(objId) { + const obj = this._objs[objId]; + return (obj ? obj.resolved : false); + } - // If there isn't an object yet or the object isn't resolved, then the - // data isn't ready yet! - if (!obj || !obj.resolved) { - throw new Error(`Requesting object that isn't resolved yet ${objId}`); - } + hasData(objId) { + return this.isResolved(objId); + } - return obj.data; - }, + /** + * Returns the data of `objId` if the object exists, null otherwise. + */ + getData(objId) { + const obj = this._objs[objId]; + if (!obj || !obj.resolved) { + return null; + } + return obj.data; + } - /** - * Resolves the object `objId` with optional `data`. - */ - resolve: function PDFObjects_resolve(objId, data) { - var obj = this.ensureObj(objId); - - obj.resolved = true; - obj.data = data; - obj.capability.resolve(data); - }, - - isResolved: function PDFObjects_isResolved(objId) { - var objs = this.objs; - - if (!objs[objId]) { - return false; - } - return objs[objId].resolved; - }, - - hasData: function PDFObjects_hasData(objId) { - return this.isResolved(objId); - }, - - /** - * Returns the data of `objId` if object exists, null otherwise. - */ - getData: function PDFObjects_getData(objId) { - var objs = this.objs; - if (!objs[objId] || !objs[objId].resolved) { - return null; - } - return objs[objId].data; - }, - - clear: function PDFObjects_clear() { - this.objs = Object.create(null); - }, - }; - return PDFObjects; -})(); + clear() { + this._objs = Object.create(null); + } +} /** * Allows controlling of the rendering tasks. From 60da2d882bf432f474b033ccd489cd647fe125d2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 7 Nov 2018 14:36:29 +0100 Subject: [PATCH 2/2] [api-minor] Refactor/simplify the `PDFObject` class First of all, note how there's currently *two* methods for checking if a certain object exists, which seems completely unwarranted. Furthermore, the rarely used `getData` method was removed and its only callsite changed to use a combination of `PDFObjects.{has, get}` instead. Finally, the methods were rearranged slightly, to bring the most important ones (for an API user) to the top of the class. --- src/display/annotation_layer.js | 5 +++-- src/display/api.js | 29 +++++++---------------------- src/display/canvas.js | 2 +- 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 27113f818..5b66faba9 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -447,8 +447,9 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement { element.style.display = 'table-cell'; let font = null; - if (this.data.fontRefName) { - font = this.page.commonObjs.getData(this.data.fontRefName); + if (this.data.fontRefName && + this.page.commonObjs.has(this.data.fontRefName)) { + font = this.page.commonObjs.get(this.data.fontRefName); } this._setTextStyle(element, font); } diff --git a/src/display/api.js b/src/display/api.js index 784fc30ff..a3ff31530 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1885,7 +1885,7 @@ class WorkerTransport { } const [id, type, exportedData] = data; - if (this.commonObjs.hasData(id)) { + if (this.commonObjs.has(id)) { return; } @@ -1937,7 +1937,7 @@ class WorkerTransport { const [id, pageIndex, type, imageData] = data; const pageProxy = this.pageCache[pageIndex]; - if (pageProxy.objs.hasData(id)) { + if (pageProxy.objs.has(id)) { return; } @@ -2256,6 +2256,11 @@ class PDFObjects { return obj.data; } + has(objId) { + const obj = this._objs[objId]; + return (obj ? obj.resolved : false); + } + /** * Resolves the object `objId` with optional `data`. */ @@ -2267,26 +2272,6 @@ class PDFObjects { obj.capability.resolve(data); } - isResolved(objId) { - const obj = this._objs[objId]; - return (obj ? obj.resolved : false); - } - - hasData(objId) { - return this.isResolved(objId); - } - - /** - * Returns the data of `objId` if the object exists, null otherwise. - */ - getData(objId) { - const obj = this._objs[objId]; - if (!obj || !obj.resolved) { - return null; - } - return obj.data; - } - clear() { this._objs = Object.create(null); } diff --git a/src/display/canvas.js b/src/display/canvas.js index 2ea4ec31a..af39bf763 100644 --- a/src/display/canvas.js +++ b/src/display/canvas.js @@ -810,7 +810,7 @@ var CanvasGraphics = (function CanvasGraphicsClosure() { // If the promise isn't resolved yet, add the continueCallback // to the promise and bail out. - if (!objsPool.isResolved(depObjId)) { + if (!objsPool.has(depObjId)) { objsPool.get(depObjId, continueCallback); return i; }