From f2fc9ee2815b63a5ef5f38e99465a9a3d0f39b66 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 13 Jun 2017 09:56:49 +0200 Subject: [PATCH 1/3] Slightly refactor and ES6-ify the code in `ObjectLoader` This patch changes all `var` to `let`, and caches the array lengths in all loops. Also removes two unnecessary temporary variable assignments. --- src/core/obj.js | 61 +++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index 4e3e697e8..e6e114450 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -1616,32 +1616,32 @@ var FileSpec = (function FileSpecClosure() { * missing data requests and then resume from the nodes that weren't ready. * * NOTE: It provides protection from circular references by keeping track of - * of loaded references. However, you must be careful not to load any graphs + * loaded references. However, you must be careful not to load any graphs * that have references to the catalog or other pages since that will cause the * entire PDF document object graph to be traversed. */ -var ObjectLoader = (function() { +let ObjectLoader = (function() { function mayHaveChildren(value) { return isRef(value) || isDict(value) || isArray(value) || isStream(value); } function addChildren(node, nodesToVisit) { - var value; + let value; if (isDict(node) || isStream(node)) { - var map; + let map; if (isDict(node)) { map = node.map; } else { map = node.dict.map; } - for (var key in map) { + for (let key in map) { value = map[key]; if (mayHaveChildren(value)) { nodesToVisit.push(value); } } } else if (isArray(node)) { - for (var i = 0, ii = node.length; i < ii; i++) { + for (let i = 0, ii = node.length; i < ii; i++) { value = node[i]; if (mayHaveChildren(value)) { nodesToVisit.push(value); @@ -1659,8 +1659,7 @@ var ObjectLoader = (function() { } ObjectLoader.prototype = { - load: function ObjectLoader_load() { - var keys = this.keys; + load() { this.capability = createPromiseCapability(); // Don't walk the graph if all the data is already loaded. if (!(this.xref.stream instanceof ChunkedStream) || @@ -1669,10 +1668,11 @@ var ObjectLoader = (function() { return this.capability.promise; } + let keys = this.keys; this.refSet = new RefSet(); // Setup the initial nodes to visit. - var nodesToVisit = []; - for (var i = 0; i < keys.length; i++) { + let nodesToVisit = []; + for (let i = 0, ii = keys.length; i < ii; i++) { nodesToVisit.push(this.obj[keys[i]]); } @@ -1680,12 +1680,12 @@ var ObjectLoader = (function() { return this.capability.promise; }, - _walk: function ObjectLoader_walk(nodesToVisit) { - var nodesToRevisit = []; - var pendingRequests = []; + _walk(nodesToVisit) { + let nodesToRevisit = []; + let pendingRequests = []; // DFS walk of the object graph. while (nodesToVisit.length) { - var currentNode = nodesToVisit.pop(); + let currentNode = nodesToVisit.pop(); // Only references or chunked streams can cause missing data exceptions. if (isRef(currentNode)) { @@ -1694,28 +1694,24 @@ var ObjectLoader = (function() { continue; } try { - var ref = currentNode; - this.refSet.put(ref); + this.refSet.put(currentNode); currentNode = this.xref.fetch(currentNode); - } catch (e) { - if (!(e instanceof MissingDataException)) { - throw e; + } catch (ex) { + if (!(ex instanceof MissingDataException)) { + throw ex; } nodesToRevisit.push(currentNode); - pendingRequests.push({ begin: e.begin, end: e.end, }); + pendingRequests.push({ begin: ex.begin, end: ex.end, }); } } if (currentNode && currentNode.getBaseStreams) { - var baseStreams = currentNode.getBaseStreams(); - var foundMissingData = false; - for (var i = 0; i < baseStreams.length; i++) { - var stream = baseStreams[i]; + let baseStreams = currentNode.getBaseStreams(); + let foundMissingData = false; + for (let i = 0, ii = baseStreams.length; i < ii; i++) { + let stream = baseStreams[i]; if (stream.getMissingChunks && stream.getMissingChunks().length) { foundMissingData = true; - pendingRequests.push({ - begin: stream.start, - end: stream.end, - }); + pendingRequests.push({ begin: stream.start, end: stream.end, }); } } if (foundMissingData) { @@ -1728,16 +1724,15 @@ var ObjectLoader = (function() { if (pendingRequests.length) { this.xref.stream.manager.requestRanges(pendingRequests).then(() => { - nodesToVisit = nodesToRevisit; - for (var i = 0; i < nodesToRevisit.length; i++) { - var node = nodesToRevisit[i]; - // Remove any reference nodes from the currrent refset so they + for (let i = 0, ii = nodesToRevisit.length; i < ii; i++) { + let node = nodesToRevisit[i]; + // Remove any reference nodes from the current `RefSet` so they // aren't skipped when we revist them. if (isRef(node)) { this.refSet.remove(node); } } - this._walk(nodesToVisit); + this._walk(nodesToRevisit); }, this.capability.reject); return; } From 3a20fd165f1cdefe16e253c13345924d324de38a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 13 Jun 2017 10:22:11 +0200 Subject: [PATCH 2/3] Refactor `ObjectLoader` to use `Dict`s correctly, rather than abusing their internal properties The `ObjectLoader` currently takes an Object as input, despite actually working with `Dict`s internally. This means that at the (two) existing call-sites, we're passing in the "private" `Dict.map` property directly. Doing this seems like an anti-pattern, and we could (and even should) simply provide the actual `Dict` when creating an `ObjectLoader` instance. Accessing properties stored in the `Dict` is now done using the intended methods instead, in particular `getRaw` which (as the name suggests) doesn't do any de-referencing, thus maintaining the current functionality of the code. The only functional change in this patch is that `ObjectLoader.load` will now ignore empty nodes, such that `ObjectLoader._walk` only needs to deal with nodes that are known to contain data. (This lets us skip, among other checks, meaningless `addChildren` function calls.) --- src/core/annotation.js | 5 ++--- src/core/document.js | 5 ++--- src/core/obj.js | 33 ++++++++++++++++----------------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index 6f81ae6ea..b1320287c 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -390,9 +390,8 @@ var Annotation = (function AnnotationClosure() { if (!resources) { return; } - var objectLoader = new ObjectLoader(resources.map, - keys, - resources.xref); + let objectLoader = new ObjectLoader(resources, keys, resources.xref); + return objectLoader.load().then(function() { return resources; }); diff --git a/src/core/document.js b/src/core/document.js index e3ed0b97b..2aea50098 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -186,9 +186,8 @@ var Page = (function PageClosure() { this.resourcesPromise = this.pdfManager.ensure(this, 'resources'); } return this.resourcesPromise.then(() => { - var objectLoader = new ObjectLoader(this.resources.map, - keys, - this.xref); + let objectLoader = new ObjectLoader(this.resources, keys, this.xref); + return objectLoader.load(); }); }, diff --git a/src/core/obj.js b/src/core/obj.js index e6e114450..5d3b4acfa 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -1610,7 +1610,7 @@ var FileSpec = (function FileSpecClosure() { })(); /** - * A helper for loading missing data in object graphs. It traverses the graph + * A helper for loading missing data in `Dict` graphs. It traverses the graph * depth first and queues up any objects that have missing data. Once it has * has traversed as many objects that are available it attempts to bundle the * missing data requests and then resume from the nodes that weren't ready. @@ -1626,23 +1626,18 @@ let ObjectLoader = (function() { } function addChildren(node, nodesToVisit) { - let value; if (isDict(node) || isStream(node)) { - let map; - if (isDict(node)) { - map = node.map; - } else { - map = node.dict.map; - } - for (let key in map) { - value = map[key]; - if (mayHaveChildren(value)) { - nodesToVisit.push(value); + let dict = isDict(node) ? node : node.dict; + let dictKeys = dict.getKeys(); + for (let i = 0, ii = dictKeys.length; i < ii; i++) { + let rawValue = dict.getRaw(dictKeys[i]); + if (mayHaveChildren(rawValue)) { + nodesToVisit.push(rawValue); } } } else if (isArray(node)) { for (let i = 0, ii = node.length; i < ii; i++) { - value = node[i]; + let value = node[i]; if (mayHaveChildren(value)) { nodesToVisit.push(value); } @@ -1650,8 +1645,8 @@ let ObjectLoader = (function() { } } - function ObjectLoader(obj, keys, xref) { - this.obj = obj; + function ObjectLoader(dict, keys, xref) { + this.dict = dict; this.keys = keys; this.xref = xref; this.refSet = null; @@ -1668,12 +1663,16 @@ let ObjectLoader = (function() { return this.capability.promise; } - let keys = this.keys; + let { keys, dict, } = this; this.refSet = new RefSet(); // Setup the initial nodes to visit. let nodesToVisit = []; for (let i = 0, ii = keys.length; i < ii; i++) { - nodesToVisit.push(this.obj[keys[i]]); + let rawValue = dict.getRaw(keys[i]); + // Skip nodes that are guaranteed to be empty. + if (rawValue !== undefined) { + nodesToVisit.push(rawValue); + } } this._walk(nodesToVisit); From 73234577e14040d4cdc999457ca7e04fcdf0a6e3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 13 Jun 2017 10:25:33 +0200 Subject: [PATCH 3/3] Rename `map` to `_map` inside of `Dict`, to make it clearer that it should be regarded as a "private" property --- src/core/primitives.js | 46 +++++++++++++++++++-------------------- src/shared/fonts_utils.js | 6 ++--- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/core/primitives.js b/src/core/primitives.js index 00b8893ca..b709762d9 100644 --- a/src/core/primitives.js +++ b/src/core/primitives.js @@ -60,7 +60,7 @@ var Dict = (function DictClosure() { // xref is optional function Dict(xref) { // Map should only be used internally, use functions below to access. - this.map = Object.create(null); + this._map = Object.create(null); this.xref = xref; this.objId = null; this.suppressEncryption = false; @@ -76,15 +76,15 @@ var Dict = (function DictClosure() { get: function Dict_get(key1, key2, key3) { var value; var xref = this.xref, suppressEncryption = this.suppressEncryption; - if (typeof (value = this.map[key1]) !== 'undefined' || key1 in this.map || - typeof key2 === 'undefined') { + if (typeof (value = this._map[key1]) !== 'undefined' || + key1 in this._map || typeof key2 === 'undefined') { return xref ? xref.fetchIfRef(value, suppressEncryption) : value; } - if (typeof (value = this.map[key2]) !== 'undefined' || key2 in this.map || - typeof key3 === 'undefined') { + if (typeof (value = this._map[key2]) !== 'undefined' || + key2 in this._map || typeof key3 === 'undefined') { return xref ? xref.fetchIfRef(value, suppressEncryption) : value; } - value = this.map[key3] || null; + value = this._map[key3] || null; return xref ? xref.fetchIfRef(value, suppressEncryption) : value; }, @@ -92,21 +92,21 @@ var Dict = (function DictClosure() { getAsync: function Dict_getAsync(key1, key2, key3) { var value; var xref = this.xref, suppressEncryption = this.suppressEncryption; - if (typeof (value = this.map[key1]) !== 'undefined' || key1 in this.map || - typeof key2 === 'undefined') { + if (typeof (value = this._map[key1]) !== 'undefined' || + key1 in this._map || typeof key2 === 'undefined') { if (xref) { return xref.fetchIfRefAsync(value, suppressEncryption); } return Promise.resolve(value); } - if (typeof (value = this.map[key2]) !== 'undefined' || key2 in this.map || - typeof key3 === 'undefined') { + if (typeof (value = this._map[key2]) !== 'undefined' || + key2 in this._map || typeof key3 === 'undefined') { if (xref) { return xref.fetchIfRefAsync(value, suppressEncryption); } return Promise.resolve(value); } - value = this.map[key3] || null; + value = this._map[key3] || null; if (xref) { return xref.fetchIfRefAsync(value, suppressEncryption); } @@ -132,23 +132,23 @@ var Dict = (function DictClosure() { // no dereferencing getRaw: function Dict_getRaw(key) { - return this.map[key]; + return this._map[key]; }, getKeys: function Dict_getKeys() { - return Object.keys(this.map); + return Object.keys(this._map); }, set: function Dict_set(key, value) { - this.map[key] = value; + this._map[key] = value; }, has: function Dict_has(key) { - return key in this.map; + return key in this._map; }, forEach: function Dict_forEach(callback) { - for (var key in this.map) { + for (var key in this._map) { callback(key, this.get(key)); } }, @@ -156,19 +156,19 @@ var Dict = (function DictClosure() { Dict.empty = new Dict(null); - Dict.merge = function Dict_merge(xref, dictArray) { - var mergedDict = new Dict(xref); + Dict.merge = function(xref, dictArray) { + let mergedDict = new Dict(xref); - for (var i = 0, ii = dictArray.length; i < ii; i++) { - var dict = dictArray[i]; + for (let i = 0, ii = dictArray.length; i < ii; i++) { + let dict = dictArray[i]; if (!isDict(dict)) { continue; } - for (var keyName in dict.map) { - if (mergedDict.map[keyName]) { + for (let keyName in dict._map) { + if (mergedDict._map[keyName] !== undefined) { continue; } - mergedDict.map[keyName] = dict.map[keyName]; + mergedDict._map[keyName] = dict._map[keyName]; } } return mergedDict; diff --git a/src/shared/fonts_utils.js b/src/shared/fonts_utils.js index 555b18e19..d9a24f662 100644 --- a/src/shared/fonts_utils.js +++ b/src/shared/fonts_utils.js @@ -361,9 +361,9 @@ var Type2Parser = function type2Parser(aFilePath) { dump('privateData:' + privateDict); parseAsToken(privateDict, CFFDictPrivateDataMap); - for (var p in font.map) { - dump(p + '::' + font.get(p)); - } + font.forEach(function(key, value) { + dump(key + '::' + value); + }); // Read CharStrings Index var charStringsOffset = font.get('CharStrings');